[llvm] 99ef341 - [InstCombine] freeze operand in sdiv expansion

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2022-05-11T14:01:28-04:00
New Revision: 99ef341ce9436e0471eb5502a8521fcaddaf851c

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

LOG: [InstCombine] freeze operand in sdiv expansion

As discussed in issue #37809, this transform is not safe
if the input is an undefined value.

This is similar to a recent change for urem:
d428f09b2c9d

There is no difference in codegen on the basic examples,
but this could lead to regressions. We may need to
improve freeze analysis or lowering if that happens.

Presumably, in real cases that are similar to the tests
where a subsequent transform removes the select, we
will also be able to remove the freeze by seeing that
the parameter has 'noundef'.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
    llvm/test/Transforms/InstCombine/div.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
index bcf4e66e9a69..e185da6df52e 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -849,12 +849,13 @@ Instruction *InstCombinerImpl::commonIDivTransforms(BinaryOperator &I) {
   if (match(Op0, m_One())) {
     assert(!Ty->isIntOrIntVectorTy(1) && "i1 divide not removed?");
     if (IsSigned) {
-      // If Op1 is 0 then it's undefined behaviour, if Op1 is 1 then the
-      // result is one, if Op1 is -1 then the result is minus one, otherwise
-      // it's zero.
-      Value *Inc = Builder.CreateAdd(Op1, Op0);
+      // 1 / 0 --> undef ; 1 / 1 --> 1 ; 1 / -1 --> -1 ; 1 / anything else --> 0
+      // (Op1 + 1) u< 3 ? Op1 : 0
+      // Op1 must be frozen because we are increasing its number of uses.
+      Value *F1 = Builder.CreateFreeze(Op1, Op1->getName() + ".fr");
+      Value *Inc = Builder.CreateAdd(F1, Op0);
       Value *Cmp = Builder.CreateICmpULT(Inc, ConstantInt::get(Ty, 3));
-      return SelectInst::Create(Cmp, Op1, ConstantInt::get(Ty, 0));
+      return SelectInst::Create(Cmp, F1, ConstantInt::get(Ty, 0));
     } else {
       // If Op1 is 0 then it's undefined behaviour. If Op1 is 1 then the
       // result is one, otherwise it's zero.

diff  --git a/llvm/test/Transforms/InstCombine/div.ll b/llvm/test/Transforms/InstCombine/div.ll
index c34b737f0fb8..bfb83956b4e4 100644
--- a/llvm/test/Transforms/InstCombine/div.ll
+++ b/llvm/test/Transforms/InstCombine/div.ll
@@ -296,9 +296,10 @@ define <2 x i32> @test19vec(<2 x i32> %x) {
 
 define i32 @test20(i32 %x) {
 ; CHECK-LABEL: @test20(
-; CHECK-NEXT:    [[TMP1:%.*]] = add i32 [[X:%.*]], 1
+; CHECK-NEXT:    [[X_FR:%.*]] = freeze i32 [[X:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = add i32 [[X_FR]], 1
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp ult i32 [[TMP1]], 3
-; CHECK-NEXT:    [[A:%.*]] = select i1 [[TMP2]], i32 [[X]], i32 0
+; CHECK-NEXT:    [[A:%.*]] = select i1 [[TMP2]], i32 [[X_FR]], i32 0
 ; CHECK-NEXT:    ret i32 [[A]]
 ;
   %A = sdiv i32 1, %x
@@ -307,9 +308,10 @@ define i32 @test20(i32 %x) {
 
 define <2 x i32> @test20vec(<2 x i32> %x) {
 ; CHECK-LABEL: @test20vec(
-; CHECK-NEXT:    [[TMP1:%.*]] = add <2 x i32> [[X:%.*]], <i32 1, i32 1>
+; CHECK-NEXT:    [[X_FR:%.*]] = freeze <2 x i32> [[X:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = add <2 x i32> [[X_FR]], <i32 1, i32 1>
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp ult <2 x i32> [[TMP1]], <i32 3, i32 3>
-; CHECK-NEXT:    [[A:%.*]] = select <2 x i1> [[TMP2]], <2 x i32> [[X]], <2 x i32> zeroinitializer
+; CHECK-NEXT:    [[A:%.*]] = select <2 x i1> [[TMP2]], <2 x i32> [[X_FR]], <2 x i32> zeroinitializer
 ; CHECK-NEXT:    ret <2 x i32> [[A]]
 ;
   %A = sdiv <2 x i32> <i32 1, i32 1>, %x
@@ -723,9 +725,10 @@ define <2 x i8> @PR34841(<2 x i8> %x) {
 
 define i8 @div_factor_signed(i8 %x, i8 %y) {
 ; CHECK-LABEL: @div_factor_signed(
-; CHECK-NEXT:    [[TMP1:%.*]] = add i8 [[Y:%.*]], 1
+; CHECK-NEXT:    [[Y_FR:%.*]] = freeze i8 [[Y:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = add i8 [[Y_FR]], 1
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp ult i8 [[TMP1]], 3
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[TMP2]], i8 [[Y]], i8 0
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[TMP2]], i8 [[Y_FR]], i8 0
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %a = mul nsw i8 %x, %y
@@ -737,9 +740,10 @@ define i8 @div_factor_signed(i8 %x, i8 %y) {
 
 define <2 x i8> @div_factor_signed_vec(<2 x i8> %x, <2 x i8> %y) {
 ; CHECK-LABEL: @div_factor_signed_vec(
-; CHECK-NEXT:    [[TMP1:%.*]] = add <2 x i8> [[Y:%.*]], <i8 1, i8 1>
+; CHECK-NEXT:    [[Y_FR:%.*]] = freeze <2 x i8> [[Y:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = add <2 x i8> [[Y_FR]], <i8 1, i8 1>
 ; CHECK-NEXT:    [[TMP2:%.*]] = icmp ult <2 x i8> [[TMP1]], <i8 3, i8 3>
-; CHECK-NEXT:    [[R:%.*]] = select <2 x i1> [[TMP2]], <2 x i8> [[Y]], <2 x i8> zeroinitializer
+; CHECK-NEXT:    [[R:%.*]] = select <2 x i1> [[TMP2]], <2 x i8> [[Y_FR]], <2 x i8> zeroinitializer
 ; CHECK-NEXT:    ret <2 x i8> [[R]]
 ;
   %a = mul nsw <2 x i8> %y, %x
@@ -1303,7 +1307,8 @@ define i32 @udiv_select_of_constants_divisor(i1 %b, i32 %x) {
 
 define i1 @sdiv_one_icmpeq_one(i32 %x) {
 ; CHECK-LABEL: @sdiv_one_icmpeq_one(
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i32 [[X:%.*]], 1
+; CHECK-NEXT:    [[X_FR:%.*]] = freeze i32 [[X:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i32 [[X_FR]], 1
 ; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %A = sdiv i32 1, %x
@@ -1313,7 +1318,8 @@ define i1 @sdiv_one_icmpeq_one(i32 %x) {
 
 define i1 @sdiv_one_icmpeq_negone(i32 %x) {
 ; CHECK-LABEL: @sdiv_one_icmpeq_negone(
-; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i32 [[X:%.*]], -1
+; CHECK-NEXT:    [[X_FR:%.*]] = freeze i32 [[X:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i32 [[X_FR]], -1
 ; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %A = sdiv i32 1, %x