[Mlir-commits] [mlir] 018645b - Fix side-effect detection in LLVMIRIntrinsicGen.

[Mehdi Amini]

Author: Andrew Pritchard
Date: 2021-02-10T00:48:16Z
New Revision: 018645b81b3bc4485bae8e9bc6e904ecd4c4a5ec

URL: https://github.com/llvm/llvm-project/commit/018645b81b3bc4485bae8e9bc6e904ecd4c4a5ec
DIFF: https://github.com/llvm/llvm-project/commit/018645b81b3bc4485bae8e9bc6e904ecd4c4a5ec.diff

LOG: Fix side-effect detection in LLVMIRIntrinsicGen.

Previously it reported an op had side-effects iff it declared that it
didn't have any side-effects.  This had the undesirable result that
canonicalization would always delete any intrinsic calls that did memory
stores and returned void.

Reviewed By: ftynse, mehdi_amini

Differential Revision: https://reviews.llvm.org/D96369

Added: 
    

Modified: 
    mlir/test/mlir-tblgen/llvm-intrinsics.td
    mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/test/mlir-tblgen/llvm-intrinsics.td b/mlir/test/mlir-tblgen/llvm-intrinsics.td
index 3ac785c2e477..511f062e8e4a 100644
--- a/mlir/test/mlir-tblgen/llvm-intrinsics.td
+++ b/mlir/test/mlir-tblgen/llvm-intrinsics.td
@@ -20,8 +20,8 @@
 // The second operand is overloadable, but the first operand needs to
 // match the result type.
 // CHECK: [1]
-// It has no additional traits.
-// CHECK: []
+// It has no side effects.
+// CHECK: [NoSideEffect]
 // It has a result.
 // CHECK: 1>
 // CHECK: Arguments<(ins LLVM_Type, LLVM_Type

diff  --git a/mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp b/mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp
index e697f6556851..72554a1f0c24 100644
--- a/mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp
+++ b/mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp
@@ -140,23 +140,17 @@ class LLVMIntrinsic {
   /// Return true if the intrinsic may have side effects, i.e. does not have the
   /// `IntrNoMem` property.
   bool hasSideEffects() const {
-    auto props = record.getValueAsListOfDefs(fieldTraits);
-    for (const llvm::Record *r : props) {
-      if (r->getName() == "IntrNoMem")
-        return true;
-    }
-    return false;
+    return llvm::none_of(
+        record.getValueAsListOfDefs(fieldTraits),
+        [](const llvm::Record *r) { return r->getName() == "IntrNoMem"; });
   }
 
   /// Return true if the intrinsic is commutative, i.e. has the respective
   /// property.
   bool isCommutative() const {
-    auto props = record.getValueAsListOfDefs(fieldTraits);
-    for (const llvm::Record *r : props) {
-      if (r->getName() == "Commutative")
-        return true;
-    }
-    return false;
+    return llvm::any_of(
+        record.getValueAsListOfDefs(fieldTraits),
+        [](const llvm::Record *r) { return r->getName() == "Commutative"; });
   }
 
   IndicesTy getOverloadableOperandsIdxs() const {