[Backport][MLIR] Properties.td fix from main commit 77f2560

[yu810226]

This PR backports the change from commit 77f2560
on main to release/21.x. Only mlir/include/mlir/IR/Properties.td is affected.

This backport is requested for the LLVM 21 release to resolve the MLIR bytecode corruption issue.
Thanks to the original author, @fabianmcg , for the fix!

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-mlir-core

Author: Lin-Ya Yu (yu810226)

Changes

This PR backports the change from commit 77f2560
on main to release/21.x. Only mlir/include/mlir/IR/Properties.td is affected.

Original commit: 77f2560

(cherry picked manually)

---

Full diff: https://github.com/llvm/llvm-project/pull/165768.diff

1 Files Affected:

• (modified) mlir/include/mlir/IR/Properties.td (+4-3)

diff --git a/mlir/include/mlir/IR/Properties.td b/mlir/include/mlir/IR/Properties.td
index a6221f9aaaef9..a7ade0675b9bb 100644
--- a/mlir/include/mlir/IR/Properties.td
+++ b/mlir/include/mlir/IR/Properties.td
@@ -773,9 +773,10 @@ class OptionalProp<Property p, bit canDelegateParsing = 1>
   }];
   let writeToMlirBytecode = [{
     $_writer.writeOwnedBool($_storage.has_value());
-    if (!$_storage.has_value())
-      return;
-  }] # !subst("$_storage", "(*($_storage))", p.writeToMlirBytecode);
+    if ($_storage.has_value()) {
+      }] # !subst("$_storage", "(*($_storage))", p.writeToMlirBytecode) # [{
+    }
+  }];
 
   let hashProperty = !if(!empty(p.hashProperty), p.hashProperty,
     [{ hash_value($_storage.has_value() ? std::optional<::llvm::hash_code>{}] #

[llvmbot]

@llvm/pr-subscribers-mlir-ods

Author: Lin-Ya Yu (yu810226)

Changes

This PR backports the change from commit 77f2560
on main to release/21.x. Only mlir/include/mlir/IR/Properties.td is affected.

Original commit: 77f2560

(cherry picked manually)

---

Full diff: https://github.com/llvm/llvm-project/pull/165768.diff

1 Files Affected:

• (modified) mlir/include/mlir/IR/Properties.td (+4-3)

diff --git a/mlir/include/mlir/IR/Properties.td b/mlir/include/mlir/IR/Properties.td
index a6221f9aaaef9..a7ade0675b9bb 100644
--- a/mlir/include/mlir/IR/Properties.td
+++ b/mlir/include/mlir/IR/Properties.td
@@ -773,9 +773,10 @@ class OptionalProp<Property p, bit canDelegateParsing = 1>
   }];
   let writeToMlirBytecode = [{
     $_writer.writeOwnedBool($_storage.has_value());
-    if (!$_storage.has_value())
-      return;
-  }] # !subst("$_storage", "(*($_storage))", p.writeToMlirBytecode);
+    if ($_storage.has_value()) {
+      }] # !subst("$_storage", "(*($_storage))", p.writeToMlirBytecode) # [{
+    }
+  }];
 
   let hashProperty = !if(!empty(p.hashProperty), p.hashProperty,
     [{ hash_value($_storage.has_value() ? std::optional<::llvm::hash_code>{}] #

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Lin-Ya Yu (yu810226)

Changes

This PR backports the change from commit 77f2560
on main to release/21.x. Only mlir/include/mlir/IR/Properties.td is affected.

Original commit: 77f2560

(cherry picked manually)

---

Full diff: https://github.com/llvm/llvm-project/pull/165768.diff

1 Files Affected:

• (modified) mlir/include/mlir/IR/Properties.td (+4-3)

diff --git a/mlir/include/mlir/IR/Properties.td b/mlir/include/mlir/IR/Properties.td
index a6221f9aaaef9..a7ade0675b9bb 100644
--- a/mlir/include/mlir/IR/Properties.td
+++ b/mlir/include/mlir/IR/Properties.td
@@ -773,9 +773,10 @@ class OptionalProp<Property p, bit canDelegateParsing = 1>
   }];
   let writeToMlirBytecode = [{
     $_writer.writeOwnedBool($_storage.has_value());
-    if (!$_storage.has_value())
-      return;
-  }] # !subst("$_storage", "(*($_storage))", p.writeToMlirBytecode);
+    if ($_storage.has_value()) {
+      }] # !subst("$_storage", "(*($_storage))", p.writeToMlirBytecode) # [{
+    }
+  }];
 
   let hashProperty = !if(!empty(p.hashProperty), p.hashProperty,
     [{ hash_value($_storage.has_value() ? std::optional<::llvm::hash_code>{}] #

[joker-eph]

@fabianmcg as the author of the original commit

[fabianmcg]

LGTM!

[fabianmcg]

The only issue I can raise is,this is untested in this patch (I'm not familiar with back port requirements on tests, so it'd be great if someone can comment on that). In the other PR, the issue was indirectly tested by the ptr op.

[yu810226]

Thanks for the review!

This is a clean backport of the fix from main. As pointed out, the issue is indirectly tested in the original commit (via the ptr op), so I didn’t backport the tests here.
If anyone familiar with the backport testing process could comment on the test coverage question (raised above) or help with merging, that’d be much appreciated. I don’t have write access myself.