[llvm] bdd5bfd - [IR][GVN] add/allow commutative intrinsics with >2 args

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2020-09-03T10:14:53-04:00
New Revision: bdd5bfd0e434637c44916fe2072b1d80fa022092

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

LOG: [IR][GVN] add/allow commutative intrinsics with >2 args

Follow-up to D86798 and rGe25449f.

Added: 
    

Modified: 
    llvm/include/llvm/IR/IntrinsicInst.h
    llvm/lib/Transforms/Scalar/GVN.cpp
    llvm/test/Transforms/GVN/commute.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index c29d20c1729b..9ba9ea68f989 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -70,13 +70,12 @@ class IntrinsicInst : public CallInst {
     case Intrinsic::uadd_with_overflow:
     case Intrinsic::smul_with_overflow:
     case Intrinsic::umul_with_overflow:
-    // TODO: These fixed-point math intrinsics have commutative first two
-    //       operands, but callers may not handle instructions with more than
-    //       two operands.
-    // case Intrinsic::smul_fix:
-    // case Intrinsic::umul_fix:
-    // case Intrinsic::smul_fix_sat:
-    // case Intrinsic::umul_fix_sat:
+    case Intrinsic::smul_fix:
+    case Intrinsic::umul_fix:
+    case Intrinsic::smul_fix_sat:
+    case Intrinsic::umul_fix_sat:
+    case Intrinsic::fma:
+    case Intrinsic::fmuladd:
       return true;
     default:
       return false;

diff  --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index f8962c085224..c71038d66f99 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -295,9 +295,7 @@ GVN::Expression GVN::ValueTable::createExpr(Instruction *I) {
     // of their operands get the same value number by sorting the operand value
     // numbers.  Since commutative operands are the 1st two operands it is more
     // efficient to sort by hand rather than using, say, std::sort.
-    assert(((isa<BinaryOperator>(I) && I->getNumOperands() == 2) ||
-            (isa<IntrinsicInst>(I) && I->getNumOperands() == 3))
-            && "Unsupported commutative instruction!");
+    assert(I->getNumOperands() >= 2 && "Unsupported commutative instruction!");
     if (e.varargs[0] > e.varargs[1])
       std::swap(e.varargs[0], e.varargs[1]);
     e.commutative = true;
@@ -1840,9 +1838,7 @@ uint32_t GVN::ValueTable::phiTranslateImpl(const BasicBlock *Pred,
   }
 
   if (Exp.commutative) {
-    assert((Exp.varargs.size() == 2 ||
-            (Exp.opcode == Instruction::Call && Exp.varargs.size() == 3))
-            && "Unsupported commutative instruction!");
+    assert(Exp.varargs.size() >= 2 && "Unsupported commutative instruction!");
     if (Exp.varargs[0] > Exp.varargs[1]) {
       std::swap(Exp.varargs[0], Exp.varargs[1]);
       uint32_t Opcode = Exp.opcode >> 8;

diff  --git a/llvm/test/Transforms/GVN/commute.ll b/llvm/test/Transforms/GVN/commute.ll
index d0f26f6e27e6..c76318db56a4 100644
--- a/llvm/test/Transforms/GVN/commute.ll
+++ b/llvm/test/Transforms/GVN/commute.ll
@@ -74,9 +74,7 @@ declare i16 @llvm.umul.fix.i16(i16, i16, i32)
 define i16 @intrinsic_3_args(i16 %x, i16 %y) {
 ; CHECK-LABEL: @intrinsic_3_args(
 ; CHECK-NEXT:    [[M1:%.*]] = call i16 @llvm.smul.fix.i16(i16 [[X:%.*]], i16 [[Y:%.*]], i32 1)
-; CHECK-NEXT:    [[M2:%.*]] = call i16 @llvm.smul.fix.i16(i16 [[Y]], i16 [[X]], i32 1)
-; CHECK-NEXT:    [[R:%.*]] = sub i16 [[M1]], [[M2]]
-; CHECK-NEXT:    ret i16 [[R]]
+; CHECK-NEXT:    ret i16 0
 ;
   %m1 = call i16 @llvm.smul.fix.i16(i16 %x, i16 %y, i32 1)
   %m2 = call i16 @llvm.smul.fix.i16(i16 %y, i16 %x, i32 1)
@@ -84,6 +82,8 @@ define i16 @intrinsic_3_args(i16 %x, i16 %y) {
   ret i16 %r
 }
 
+; Negative test - 3rd arg is 
diff erent
+
 define i16 @intrinsic_3_args_not_same(i16 %x, i16 %y) {
 ; CHECK-LABEL: @intrinsic_3_args_not_same(
 ; CHECK-NEXT:    [[M1:%.*]] = call i16 @llvm.umul.fix.i16(i16 [[X:%.*]], i16 [[Y:%.*]], i32 2)
@@ -102,9 +102,7 @@ declare float @llvm.fma.f32(float, float, float)
 define float @fma(float %x, float %y) {
 ; CHECK-LABEL: @fma(
 ; CHECK-NEXT:    [[M1:%.*]] = call float @llvm.fma.f32(float [[X:%.*]], float [[Y:%.*]], float 1.000000e+00)
-; CHECK-NEXT:    [[M2:%.*]] = call float @llvm.fma.f32(float [[Y]], float [[X]], float 1.000000e+00)
-; CHECK-NEXT:    [[R:%.*]] = fdiv nnan float [[M1]], [[M2]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    ret float 1.000000e+00
 ;
   %m1 = call float @llvm.fma.f32(float %x, float %y, float 1.0)
   %m2 = call float @llvm.fma.f32(float %y, float %x, float 1.0)