[llvm] 30420bc - [IRSim] Make sure that commutative intrinsics are treated as function calls without commutativity

[Andrew Litteken via llvm-commits]

Author: Andrew Litteken
Date: 2022-02-02T13:24:56-06:00
New Revision: 30420bc344151eaea6fa9d32cd23a64feee5401b

URL: https://github.com/llvm/llvm-project/commit/30420bc344151eaea6fa9d32cd23a64feee5401b
DIFF: https://github.com/llvm/llvm-project/commit/30420bc344151eaea6fa9d32cd23a64feee5401b.diff

LOG: [IRSim] Make sure that commutative intrinsics are treated as function calls without commutativity

Created to fix: https://github.com/llvm/llvm-project/issues/53537

Some intrinsics functions are considered commutative since they are performing operations like addition or multiplication. Some of these have extra parameters to provide extra information that are not part of the operation itself and are not commutative. This makes sure that if an instruction that is an intrinsic takes the non commutative path to handle this case.

Reviewer: paquette

Closes Issue #53537

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

Added: 
    

Modified: 
    llvm/lib/Analysis/IRSimilarityIdentifier.cpp
    llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
index 01681c47418a..92a8e7422e19 100644
--- a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
+++ b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
@@ -791,7 +791,8 @@ bool IRSimilarityCandidate::compareStructure(
     // We have 
diff erent paths for commutative instructions and non-commutative
     // instructions since commutative instructions could allow multiple mappings
     // to certain values.
-    if (IA->isCommutative() && !isa<FPMathOperator>(IA)) {
+    if (IA->isCommutative() && !isa<FPMathOperator>(IA) &&
+        !isa<IntrinsicInst>(IA)) {
       if (!compareCommutativeOperandMapping(
               {A, OperValsA, ValueNumberMappingA},
               {B, OperValsB, ValueNumberMappingB}))

diff  --git a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
index 91a0594456a9..3b8bd74354f2 100644
--- a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
+++ b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
@@ -2608,6 +2608,39 @@ TEST(IRSimilarityIdentifier, CommutativeSimilarity) {
   }
 }
 
+// This test makes sure that intrinsic functions that are marked commutative
+// are still treated as non-commutative since they are function calls.
+TEST(IRSimilarityIdentifier, InstrinsicCommutative) {
+  // If treated as commutative, we will fail to find a valid mapping, causing
+  // an assertion error.
+  StringRef ModuleString = R"(
+  define void @foo() {
+    entry:
+      %0 = call i16 @llvm.smul.fix.i16(i16 16384, i16 16384, i32 15)
+      store i16 %0, i16* undef, align 1
+      %1 = icmp eq i16 undef, 8192
+      call void @bar()
+      %2 = call i16 @llvm.smul.fix.i16(i16 -16384, i16 16384, i32 15)
+      store i16 %2, i16* undef, align 1
+      %3 = icmp eq i16 undef, -8192
+      call void @bar()
+      %4 = call i16 @llvm.smul.fix.i16(i16 -16384, i16 -16384, i32 15)
+      ret void
+  }
+
+  declare void @bar()
+
+  ; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+  declare i16 @llvm.smul.fix.i16(i16, i16, i32 immarg))";
+  LLVMContext Context;
+  std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
+
+  std::vector<std::vector<IRSimilarityCandidate>> SimilarityCandidates;
+  getSimilarities(*M, SimilarityCandidates);
+
+  ASSERT_TRUE(SimilarityCandidates.size() == 0);
+}
+
 // This test checks to see whether we can detect 
diff erent structure in
 // commutative instructions.  In this case, the second operand in the second
 // add is 
diff erent.