From 2fa8fc3d0afa00a96c11bdf655fa3e8d6707942c Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Thu, 12 May 2022 12:37:29 -0400 Subject: [PATCH] [InstCombine] freeze operand in div+mul fold As discussed in issue #37809, this transform is not safe if the input is an undefined value. This is similar to recent changes for urem and sdiv: d428f09b2c9d 99ef341ce943 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 rem, we will also be able to remove the freeze by seeing that the parameter has 'noundef'. --- .../InstCombine/InstCombineMulDivRem.cpp | 8 +++-- llvm/test/Transforms/InstCombine/add4.ll | 7 +++-- llvm/test/Transforms/InstCombine/exact.ll | 14 +++++---- llvm/test/Transforms/InstCombine/rem.ll | 31 ++++++++++++------- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp index e185da6df52e..044f12bd7af9 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp @@ -292,10 +292,12 @@ Instruction *InstCombinerImpl::visitMul(BinaryOperator &I) { auto RemOpc = Div->getOpcode() == Instruction::UDiv ? Instruction::URem : Instruction::SRem; - Value *Rem = Builder.CreateBinOp(RemOpc, X, DivOp1); + // X must be frozen because we are increasing its number of uses. + Value *XFreeze = Builder.CreateFreeze(X, X->getName() + ".fr"); + Value *Rem = Builder.CreateBinOp(RemOpc, XFreeze, DivOp1); if (DivOp1 == Y) - return BinaryOperator::CreateSub(X, Rem); - return BinaryOperator::CreateSub(Rem, X); + return BinaryOperator::CreateSub(XFreeze, Rem); + return BinaryOperator::CreateSub(Rem, XFreeze); } } diff --git a/llvm/test/Transforms/InstCombine/add4.ll b/llvm/test/Transforms/InstCombine/add4.ll index 4362194c4ea0..7773aa07c76d 100644 --- a/llvm/test/Transforms/InstCombine/add4.ll +++ b/llvm/test/Transforms/InstCombine/add4.ll @@ -113,9 +113,10 @@ define i64 @not_match_inconsistent_values(i64 %x) { define i32 @not_match_overflow(i32 %x) { ; CHECK-LABEL: @not_match_overflow( -; CHECK-NEXT: [[T:%.*]] = urem i32 [[X:%.*]], 299 -; CHECK-NEXT: [[TMP1:%.*]] = urem i32 [[X]], 299 -; CHECK-NEXT: [[T3:%.*]] = sub i32 [[X]], [[TMP1]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i32 [[X:%.*]] +; CHECK-NEXT: [[T:%.*]] = urem i32 [[X_FR]], 299 +; CHECK-NEXT: [[TMP1:%.*]] = urem i32 [[X_FR]], 299 +; CHECK-NEXT: [[T3:%.*]] = sub i32 [[X_FR]], [[TMP1]] ; CHECK-NEXT: [[T4:%.*]] = add i32 [[T]], [[T3]] ; CHECK-NEXT: ret i32 [[T4]] ; diff --git a/llvm/test/Transforms/InstCombine/exact.ll b/llvm/test/Transforms/InstCombine/exact.ll index be044153f081..5e361c31d26a 100644 --- a/llvm/test/Transforms/InstCombine/exact.ll +++ b/llvm/test/Transforms/InstCombine/exact.ll @@ -30,8 +30,9 @@ define <2 x i32> @sdiv2_vec(<2 x i32> %x) { define i32 @sdiv3(i32 %x) { ; CHECK-LABEL: @sdiv3( -; CHECK-NEXT: [[TMP1:%.*]] = srem i32 [[X:%.*]], 3 -; CHECK-NEXT: [[Z:%.*]] = sub i32 [[X]], [[TMP1]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i32 [[X:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = srem i32 [[X_FR]], 3 +; CHECK-NEXT: [[Z:%.*]] = sub i32 [[X_FR]], [[TMP1]] ; CHECK-NEXT: ret i32 [[Z]] ; %y = sdiv i32 %x, 3 @@ -50,8 +51,9 @@ define i32 @sdiv4(i32 %x) { define i32 @sdiv5(i32 %x) { ; CHECK-LABEL: @sdiv5( -; CHECK-NEXT: [[TMP1:%.*]] = srem i32 [[X:%.*]], 3 -; CHECK-NEXT: [[Z:%.*]] = sub i32 [[TMP1]], [[X]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i32 [[X:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = srem i32 [[X_FR]], 3 +; CHECK-NEXT: [[Z:%.*]] = sub i32 [[TMP1]], [[X_FR]] ; CHECK-NEXT: ret i32 [[Z]] ; %y = sdiv i32 %x, 3 @@ -80,8 +82,8 @@ define i32 @udiv1(i32 %x, i32 %w) { define i32 @udiv2(i32 %x, i32 %w) { ; CHECK-LABEL: @udiv2( -; CHECK-NEXT: [[Z:%.*]] = lshr exact i32 [[X:%.*]], [[W:%.*]] -; CHECK-NEXT: ret i32 [[Z]] +; CHECK-NEXT: [[Z1:%.*]] = lshr exact i32 [[X:%.*]], [[W:%.*]] +; CHECK-NEXT: ret i32 [[Z1]] ; %y = shl i32 1, %w %z = udiv exact i32 %x, %y diff --git a/llvm/test/Transforms/InstCombine/rem.ll b/llvm/test/Transforms/InstCombine/rem.ll index 044daeb4108f..6d62cb152e61 100644 --- a/llvm/test/Transforms/InstCombine/rem.ll +++ b/llvm/test/Transforms/InstCombine/rem.ll @@ -5,7 +5,8 @@ declare void @use(i32) define i64 @rem_signed(i64 %x1, i64 %y2) { ; CHECK-LABEL: @rem_signed( -; CHECK-NEXT: [[TMP1:%.*]] = srem i64 [[X1:%.*]], [[Y2:%.*]] +; CHECK-NEXT: [[X1_FR:%.*]] = freeze i64 [[X1:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = srem i64 [[X1_FR]], [[Y2:%.*]] ; CHECK-NEXT: ret i64 [[TMP1]] ; %r = sdiv i64 %x1, %y2 @@ -16,7 +17,8 @@ define i64 @rem_signed(i64 %x1, i64 %y2) { define <4 x i32> @rem_signed_vec(<4 x i32> %t, <4 x i32> %u) { ; CHECK-LABEL: @rem_signed_vec( -; CHECK-NEXT: [[TMP1:%.*]] = srem <4 x i32> [[T:%.*]], [[U:%.*]] +; CHECK-NEXT: [[T_FR:%.*]] = freeze <4 x i32> [[T:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = srem <4 x i32> [[T_FR]], [[U:%.*]] ; CHECK-NEXT: ret <4 x i32> [[TMP1]] ; %k = sdiv <4 x i32> %t, %u @@ -27,7 +29,8 @@ define <4 x i32> @rem_signed_vec(<4 x i32> %t, <4 x i32> %u) { define i64 @rem_unsigned(i64 %x1, i64 %y2) { ; CHECK-LABEL: @rem_unsigned( -; CHECK-NEXT: [[TMP1:%.*]] = urem i64 [[X1:%.*]], [[Y2:%.*]] +; CHECK-NEXT: [[X1_FR:%.*]] = freeze i64 [[X1:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = urem i64 [[X1_FR]], [[Y2:%.*]] ; CHECK-NEXT: ret i64 [[TMP1]] ; %r = udiv i64 %x1, %y2 @@ -97,7 +100,8 @@ define <2 x i4> @big_divisor_vec(<2 x i4> %x) { define i8 @urem1(i8 %x, i8 %y) { ; CHECK-LABEL: @urem1( -; CHECK-NEXT: [[TMP1:%.*]] = urem i8 [[X:%.*]], [[Y:%.*]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i8 [[X:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = urem i8 [[X_FR]], [[Y:%.*]] ; CHECK-NEXT: ret i8 [[TMP1]] ; %A = udiv i8 %x, %y @@ -108,7 +112,8 @@ define i8 @urem1(i8 %x, i8 %y) { define i8 @srem1(i8 %x, i8 %y) { ; CHECK-LABEL: @srem1( -; CHECK-NEXT: [[TMP1:%.*]] = srem i8 [[X:%.*]], [[Y:%.*]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i8 [[X:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = srem i8 [[X_FR]], [[Y:%.*]] ; CHECK-NEXT: ret i8 [[TMP1]] ; %A = sdiv i8 %x, %y @@ -119,7 +124,8 @@ define i8 @srem1(i8 %x, i8 %y) { define i8 @urem2(i8 %x, i8 %y) { ; CHECK-LABEL: @urem2( -; CHECK-NEXT: [[TMP1:%.*]] = urem i8 [[X:%.*]], [[Y:%.*]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i8 [[X:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = urem i8 [[X_FR]], [[Y:%.*]] ; CHECK-NEXT: [[C:%.*]] = sub i8 0, [[TMP1]] ; CHECK-NEXT: ret i8 [[C]] ; @@ -131,9 +137,10 @@ define i8 @urem2(i8 %x, i8 %y) { define i8 @urem3(i8 %x) { ; CHECK-LABEL: @urem3( -; CHECK-NEXT: [[TMP1:%.*]] = urem i8 [[X:%.*]], 3 -; CHECK-NEXT: [[B_NEG:%.*]] = sub i8 [[X]], [[TMP1]] -; CHECK-NEXT: [[C:%.*]] = add i8 [[B_NEG]], [[X]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i8 [[X:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = urem i8 [[X_FR]], 3 +; CHECK-NEXT: [[B_NEG:%.*]] = sub i8 [[X_FR]], [[TMP1]] +; CHECK-NEXT: [[C:%.*]] = add i8 [[B_NEG]], [[X_FR]] ; CHECK-NEXT: ret i8 [[C]] ; %A = udiv i8 %x, 3 @@ -146,7 +153,8 @@ define i8 @urem3(i8 %x) { define i32 @sdiv_mul_sdiv(i32 %x, i32 %y) { ; CHECK-LABEL: @sdiv_mul_sdiv( -; CHECK-NEXT: [[R:%.*]] = sdiv i32 [[X:%.*]], [[Y:%.*]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i32 [[X:%.*]] +; CHECK-NEXT: [[R:%.*]] = sdiv i32 [[X_FR]], [[Y:%.*]] ; CHECK-NEXT: ret i32 [[R]] ; %div = sdiv i32 %x, %y @@ -159,7 +167,8 @@ define i32 @sdiv_mul_sdiv(i32 %x, i32 %y) { define i32 @udiv_mul_udiv(i32 %x, i32 %y) { ; CHECK-LABEL: @udiv_mul_udiv( -; CHECK-NEXT: [[R:%.*]] = udiv i32 [[X:%.*]], [[Y:%.*]] +; CHECK-NEXT: [[X_FR:%.*]] = freeze i32 [[X:%.*]] +; CHECK-NEXT: [[R:%.*]] = udiv i32 [[X_FR]], [[Y:%.*]] ; CHECK-NEXT: ret i32 [[R]] ; %div = udiv i32 %x, %y