Skip to content

[InstCombine] Tighten use constraint in factorization transforms #102943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[InstCombine] Tighten use constraint in factorization transforms #102943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
620ce7b
Precommit - transform creates extra arg uses
kalxr Aug 13, 2024
f9c2f0f
[InstCombine] Tighten use constraint in factorization transforms
kalxr Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1409,7 +1409,8 @@ Instruction *InstCombinerImpl::
/// foldUsingDistributiveLaws. If that code can be made to work optimally
/// for multi-use cases or propagating nsw/nuw, then we would not need this.
static Instruction *factorizeMathWithShlOps(BinaryOperator &I,
InstCombiner::BuilderTy &Builder) {
InstCombiner::BuilderTy &Builder,
const SimplifyQuery SQ) {
// TODO: Also handle mul by doubling the shift amount?
assert((I.getOpcode() == Instruction::Add ||
I.getOpcode() == Instruction::Sub) &&
Expand All @@ -1424,6 +1425,12 @@ static Instruction *factorizeMathWithShlOps(BinaryOperator &I,
!match(Op1, m_Shl(m_Value(Y), m_Specific(ShAmt))))
return nullptr;

// This transform is only profitiable if both operations or one operation and
// the resulting add/sub can be eliminated/folded.
if (!(Op0->hasOneUse() && Op1->hasOneUse()) &&
!simplifyBinOp(I.getOpcode(), X, Y, SQ))
return nullptr;

// No-wrap propagates only when all ops have no-wrap.
bool HasNSW = I.hasNoSignedWrap() && Op0->hasNoSignedWrap() &&
Op1->hasNoSignedWrap();
Expand Down Expand Up @@ -1505,7 +1512,7 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
if (Instruction *R = foldBoxMultiply(I))
return R;

if (Instruction *R = factorizeMathWithShlOps(I, Builder))
if (Instruction *R = factorizeMathWithShlOps(I, Builder, SQ))
return R;

if (Instruction *X = foldAddWithConstant(I))
Expand Down Expand Up @@ -2166,7 +2173,7 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
}

// Try this before Negator to preserve NSW flag.
if (Instruction *R = factorizeMathWithShlOps(I, Builder))
if (Instruction *R = factorizeMathWithShlOps(I, Builder, SQ))
return R;

Constant *C;
Expand Down
14 changes: 10 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,9 +688,12 @@ static Value *tryFactorization(BinaryOperator &I, const SimplifyQuery &SQ,
// If "B op D" simplifies then it can be formed with no cost.
V = simplifyBinOp(TopLevelOpcode, B, D, SQ.getWithInstruction(&I));

// If "B op D" doesn't simplify then only go on if one of the existing
// If "B op D" doesn't simplify then only go on if both of the existing
// operations "A op' B" and "C op' D" will be zapped as no longer used.
if (!V && (LHS->hasOneUse() || RHS->hasOneUse()))
// Note that when an operation is equal to one of its operands, that
// operation is "zapped" by having never existed in the first place.
if (!V && (LHS->hasOneUse() || LHS == A || LHS == B) &&
(RHS->hasOneUse() || RHS == C || RHS == D))
V = Builder.CreateBinOp(TopLevelOpcode, B, D, RHS->getName());
if (V)
RetVal = Builder.CreateBinOp(InnerOpcode, A, V);
Expand All @@ -708,9 +711,12 @@ static Value *tryFactorization(BinaryOperator &I, const SimplifyQuery &SQ,
// If "A op C" simplifies then it can be formed with no cost.
V = simplifyBinOp(TopLevelOpcode, A, C, SQ.getWithInstruction(&I));

// If "A op C" doesn't simplify then only go on if one of the existing
// If "A op C" doesn't simplify then only go on if both of the existing
// operations "A op' B" and "C op' D" will be zapped as no longer used.
if (!V && (LHS->hasOneUse() || RHS->hasOneUse()))
// Note that when an operation is equal to one of its operands, that
// operation is "zapped" by having never existed in the first place.
if (!V && (LHS->hasOneUse() || LHS == A || LHS == B) &&
(RHS->hasOneUse() || RHS == C || RHS == D))
V = Builder.CreateBinOp(TopLevelOpcode, A, C, LHS->getName());
if (V)
RetVal = Builder.CreateBinOp(InnerOpcode, V, B);
Expand Down
54 changes: 54 additions & 0 deletions llvm/test/Transforms/InstCombine/add.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2046,6 +2046,60 @@ define i8 @mul_add_common_factor_use(i8 %x, i8 %y) {
ret i8 %a
}

; negative test - avoid creating extra uses of args

define i8 @mul_add_common_factor_use2(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @mul_add_common_factor_use2(
; CHECK-NEXT: [[M:%.*]] = mul i8 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: [[N:%.*]] = mul i8 [[X]], [[Z:%.*]]
; CHECK-NEXT: call void @use(i8 [[M]])
; CHECK-NEXT: [[A:%.*]] = add i8 [[M]], [[N]]
; CHECK-NEXT: ret i8 [[A]]
;
%m = mul i8 %x, %y
%n = mul i8 %x, %z
call void @use(i8 %m)
%a = add i8 %m, %n
ret i8 %a
}

; negative test - avoid disturbing redundant expressions for no immediate improvement

define void @mul_add_chain_common_factor_uses(i64 %a, i64 %b, i32 %c) {
; CHECK-LABEL: @mul_add_chain_common_factor_uses(
; CHECK-NEXT: [[MUL1:%.*]] = mul i64 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[SXT1:%.*]] = sext i32 [[C:%.*]] to i64
; CHECK-NEXT: [[MUL2:%.*]] = mul i64 [[B]], [[SXT1]]
; CHECK-NEXT: [[ADD1:%.*]] = add i64 [[MUL1]], [[MUL2]]
; CHECK-NEXT: call void @use(i64 [[ADD1]])
; CHECK-NEXT: [[SXT2:%.*]] = sext i32 [[C]] to i64
; CHECK-NEXT: [[MUL3:%.*]] = mul i64 [[B]], [[SXT2]]
; CHECK-NEXT: [[ADD2:%.*]] = add i64 [[ADD1]], [[MUL3]]
; CHECK-NEXT: call void @use(i64 [[ADD2]])
; CHECK-NEXT: [[SXT3:%.*]] = sext i32 [[C]] to i64
; CHECK-NEXT: [[MUL4:%.*]] = mul i64 [[B]], [[SXT3]]
; CHECK-NEXT: [[ADD3:%.*]] = add i64 [[ADD2]], [[MUL4]]
; CHECK-NEXT: call void @use(i64 [[ADD3]])
; CHECK-NEXT: call void @use(i64 [[MUL1]])
; CHECK-NEXT: ret void
;
%mul1 = mul i64 %a, %b
%sxt1 = sext i32 %c to i64
%mul2 = mul i64 %b, %sxt1
%add1 = add i64 %mul1, %mul2
call void @use(i64 %add1)
%sxt2 = sext i32 %c to i64
%mul3 = mul i64 %b, %sxt2
%add2 = add i64 %add1, %mul3
call void @use(i64 %add2)
%sxt3 = sext i32 %c to i64
%mul4 = mul i64 %b, %sxt3
%add3 = add i64 %add2, %mul4
call void @use(i64 %add3)
call void @use(i64 %mul1)
ret void
}

define i8 @not_mul(i8 %x) {
; CHECK-LABEL: @not_mul(
; CHECK-NEXT: [[TMP1:%.*]] = mul i8 [[X:%.*]], -41
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/Transforms/InstCombine/and-or.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,11 +685,11 @@ define i32 @or_or_and_noOneUse_fail1(i32 %a, i32 %b) {
; CHECK-NEXT: [[SHR:%.*]] = ashr i32 [[A:%.*]], 23
; CHECK-NEXT: [[AND:%.*]] = and i32 [[SHR]], 157
; CHECK-NEXT: call void @use2(i32 [[AND]])
; CHECK-NEXT: [[AND1:%.*]] = or i32 [[B:%.*]], 157
; CHECK-NEXT: [[OR:%.*]] = and i32 [[SHR]], [[AND1]]
; CHECK-NEXT: [[AND3:%.*]] = and i32 [[SHR]], [[B:%.*]]
; CHECK-NEXT: [[SHR8:%.*]] = lshr i32 [[B]], 23
; CHECK-NEXT: [[AND9:%.*]] = and i32 [[SHR8]], 157
; CHECK-NEXT: [[R:%.*]] = or i32 [[OR]], [[AND9]]
; CHECK-NEXT: [[TMP1:%.*]] = or i32 [[AND3]], [[AND9]]
; CHECK-NEXT: [[R:%.*]] = or i32 [[TMP1]], [[AND]]
Copy link
Contributor

@goldsteinn goldsteinn Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a minor regression.

; CHECK-NEXT: ret i32 [[R]]
;
%shr = ashr i32 %a, 23
Expand All @@ -714,9 +714,9 @@ define { i1, i1, i1, i1, i1 } @or_or_and_noOneUse_fail2(i1 %a_0, i1 %a_1, i1 %a_
; CHECK-NEXT: [[TMP3:%.*]] = and i1 [[A_1:%.*]], [[B_1:%.*]]
; CHECK-NEXT: [[TMP4:%.*]] = xor i1 [[TMP3]], true
; CHECK-NEXT: [[TMP5:%.*]] = and i1 [[TMP0]], [[A_1]]
; CHECK-NEXT: [[TMP6:%.*]] = or i1 [[A_1]], [[TMP2]]
; CHECK-NEXT: [[TMP7:%.*]] = and i1 [[TMP6]], [[B_1]]
; CHECK-NEXT: [[D:%.*]] = or i1 [[TMP7]], [[TMP5]]
; CHECK-NEXT: [[TMP6:%.*]] = and i1 [[TMP2]], [[B_1]]
; CHECK-NEXT: [[TMP7:%.*]] = or i1 [[TMP6]], [[TMP5]]
; CHECK-NEXT: [[D:%.*]] = or i1 [[TMP7]], [[TMP3]]
; CHECK-NEXT: [[DOTNOT1:%.*]] = or i1 [[TMP1]], [[TMP3]]
; CHECK-NEXT: [[TMP8:%.*]] = insertvalue { i1, i1, i1, i1, i1 } zeroinitializer, i1 [[D]], 0
; CHECK-NEXT: [[TMP9:%.*]] = insertvalue { i1, i1, i1, i1, i1 } [[TMP8]], i1 [[TMP4]], 1
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/Transforms/InstCombine/ctpop.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,9 @@ define i32 @parity_xor_extra_use(i32 %arg, i32 %arg1) {
; CHECK-NEXT: [[I:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[ARG:%.*]])
; CHECK-NEXT: [[I2:%.*]] = and i32 [[I]], 1
; CHECK-NEXT: tail call void @use(i32 [[I2]])
; CHECK-NEXT: [[TMP1:%.*]] = xor i32 [[ARG1:%.*]], [[ARG]]
; CHECK-NEXT: [[TMP2:%.*]] = call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[TMP1]])
; CHECK-NEXT: [[I5:%.*]] = and i32 [[TMP2]], 1
; CHECK-NEXT: [[I3:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[ARG1:%.*]])
; CHECK-NEXT: [[I4:%.*]] = and i32 [[I3]], 1
; CHECK-NEXT: [[I5:%.*]] = xor i32 [[I4]], [[I2]]
; CHECK-NEXT: ret i32 [[I5]]
;
%i = tail call i32 @llvm.ctpop.i32(i32 %arg)
Expand All @@ -461,9 +461,9 @@ define i32 @parity_xor_extra_use2(i32 %arg, i32 %arg1) {
; CHECK-NEXT: [[I:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[ARG1:%.*]])
; CHECK-NEXT: [[I2:%.*]] = and i32 [[I]], 1
; CHECK-NEXT: tail call void @use(i32 [[I2]])
; CHECK-NEXT: [[TMP1:%.*]] = xor i32 [[ARG1]], [[ARG:%.*]]
; CHECK-NEXT: [[TMP2:%.*]] = call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[TMP1]])
; CHECK-NEXT: [[I5:%.*]] = and i32 [[TMP2]], 1
; CHECK-NEXT: [[I3:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[ARG:%.*]])
; CHECK-NEXT: [[I4:%.*]] = and i32 [[I3]], 1
; CHECK-NEXT: [[I5:%.*]] = xor i32 [[I2]], [[I4]]
; CHECK-NEXT: ret i32 [[I5]]
;
%i = tail call i32 @llvm.ctpop.i32(i32 %arg1)
Expand Down
20 changes: 10 additions & 10 deletions llvm/test/Transforms/InstCombine/shl-factor.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ define i8 @add_shl_same_amount_nsw_extra_use1(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @add_shl_same_amount_nsw_extra_use1(
; CHECK-NEXT: [[XS:%.*]] = shl nuw nsw i8 [[X:%.*]], [[Z:%.*]]
; CHECK-NEXT: call void @use8(i8 [[XS]])
; CHECK-NEXT: [[TMP1:%.*]] = add nsw i8 [[X]], [[Y:%.*]]
; CHECK-NEXT: [[DIFF:%.*]] = shl nsw i8 [[TMP1]], [[Z]]
; CHECK-NEXT: [[YS:%.*]] = shl nuw nsw i8 [[Y:%.*]], [[Z]]
; CHECK-NEXT: [[DIFF:%.*]] = add nsw i8 [[XS]], [[YS]]
; CHECK-NEXT: ret i8 [[DIFF]]
;
%xs = shl nsw nuw i8 %x, %z
Expand All @@ -56,10 +56,10 @@ define i8 @add_shl_same_amount_nsw_extra_use1(i8 %x, i8 %y, i8 %z) {

define i8 @add_shl_same_amount_nuw_extra_use2(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @add_shl_same_amount_nuw_extra_use2(
; CHECK-NEXT: [[YS:%.*]] = shl nuw nsw i8 [[Y:%.*]], [[Z:%.*]]
; CHECK-NEXT: [[XS:%.*]] = shl nuw i8 [[X:%.*]], [[Z:%.*]]
; CHECK-NEXT: [[YS:%.*]] = shl nuw nsw i8 [[Y:%.*]], [[Z]]
; CHECK-NEXT: call void @use8(i8 [[YS]])
; CHECK-NEXT: [[TMP1:%.*]] = add nuw i8 [[X:%.*]], [[Y]]
; CHECK-NEXT: [[DIFF:%.*]] = shl nuw i8 [[TMP1]], [[Z]]
; CHECK-NEXT: [[DIFF:%.*]] = add nuw nsw i8 [[XS]], [[YS]]
; CHECK-NEXT: ret i8 [[DIFF]]
;
%xs = shl nuw i8 %x, %z
Expand Down Expand Up @@ -174,8 +174,8 @@ define i8 @sub_shl_same_amount_nsw_extra_use1(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @sub_shl_same_amount_nsw_extra_use1(
; CHECK-NEXT: [[XS:%.*]] = shl nuw nsw i8 [[X:%.*]], [[Z:%.*]]
; CHECK-NEXT: call void @use8(i8 [[XS]])
; CHECK-NEXT: [[TMP1:%.*]] = sub nsw i8 [[X]], [[Y:%.*]]
; CHECK-NEXT: [[DIFF:%.*]] = shl nsw i8 [[TMP1]], [[Z]]
; CHECK-NEXT: [[YS:%.*]] = shl nuw nsw i8 [[Y:%.*]], [[Z]]
; CHECK-NEXT: [[DIFF:%.*]] = sub nsw i8 [[XS]], [[YS]]
; CHECK-NEXT: ret i8 [[DIFF]]
;
%xs = shl nsw nuw i8 %x, %z
Expand All @@ -187,10 +187,10 @@ define i8 @sub_shl_same_amount_nsw_extra_use1(i8 %x, i8 %y, i8 %z) {

define i8 @sub_shl_same_amount_nuw_extra_use2(i8 %x, i8 %y, i8 %z) {
; CHECK-LABEL: @sub_shl_same_amount_nuw_extra_use2(
; CHECK-NEXT: [[YS:%.*]] = shl nuw nsw i8 [[Y:%.*]], [[Z:%.*]]
; CHECK-NEXT: [[XS:%.*]] = shl nuw i8 [[X:%.*]], [[Z:%.*]]
; CHECK-NEXT: [[YS:%.*]] = shl nuw nsw i8 [[Y:%.*]], [[Z]]
; CHECK-NEXT: call void @use8(i8 [[YS]])
; CHECK-NEXT: [[TMP1:%.*]] = sub nuw i8 [[X:%.*]], [[Y]]
; CHECK-NEXT: [[DIFF:%.*]] = shl nuw i8 [[TMP1]], [[Z]]
; CHECK-NEXT: [[DIFF:%.*]] = sub nuw nsw i8 [[XS]], [[YS]]
; CHECK-NEXT: ret i8 [[DIFF]]
;
%xs = shl nuw i8 %x, %z
Expand Down
Loading