Skip to content

Commit e575d84

Browse files
committed
LICM: handle BinOp in RHS correctly
1 parent 8e8f1d7 commit e575d84

File tree

2 files changed

+57
-3
lines changed

2 files changed

+57
-3
lines changed

llvm/lib/Transforms/Scalar/LICM.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2824,15 +2824,15 @@ static bool hoistBOAssociation(Instruction &I, Loop &L,
28242824
if (Opcode != Instruction::Add && Opcode != Instruction::Mul)
28252825
return false;
28262826

2827-
bool BinOpInRHS = isa<BinaryOperator>(BO->getOperand(1));
2828-
auto *BO0 = dyn_cast<BinaryOperator>(BO->getOperand(BinOpInRHS));
2827+
bool LVInRHS = L.isLoopInvariant(BO->getOperand(0));
2828+
auto *BO0 = dyn_cast<BinaryOperator>(BO->getOperand(LVInRHS));
28292829
if (!BO0 || BO0->getOpcode() != Opcode || !BO0->isAssociative() ||
28302830
BO0->hasNUsesOrMore(3))
28312831
return false;
28322832

28332833
Value *LV = BO0->getOperand(0);
28342834
Value *C1 = BO0->getOperand(1);
2835-
Value *C2 = BO->getOperand(!BinOpInRHS);
2835+
Value *C2 = BO->getOperand(!LVInRHS);
28362836

28372837
assert(BO->isCommutative() && BO0->isCommutative() &&
28382838
"Associativity implies commutativity");

llvm/test/Transforms/LICM/hoist-binop.ll

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,33 @@ loop:
142142
br label %loop
143143
}
144144

145+
; Hoist ADD and copy NUW if both ops have it.
146+
; A version where the LHS and RHS of the outer BinOp are BinOps.
147+
define void @add_nuw_twobinops(i64 %c1, i64 %c2) {
148+
; CHECK-LABEL: @add_nuw_twobinops(
149+
; CHECK-NEXT: entry:
150+
; CHECK-NEXT: [[C2_PLUS_2:%.*]] = add nuw i64 [[C2:%.*]], 2
151+
; CHECK-NEXT: [[INVARIANT_OP:%.*]] = add nuw i64 [[C1:%.*]], [[C2_PLUS_2]]
152+
; CHECK-NEXT: br label [[LOOP:%.*]]
153+
; CHECK: loop:
154+
; CHECK-NEXT: [[INDEX:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INDEX_NEXT_REASS:%.*]], [[LOOP]] ]
155+
; CHECK-NEXT: [[STEP_ADD:%.*]] = add nuw i64 [[C1]], [[INDEX]]
156+
; CHECK-NEXT: call void @use(i64 [[STEP_ADD]])
157+
; CHECK-NEXT: [[INDEX_NEXT_REASS]] = add nuw i64 [[INDEX]], [[INVARIANT_OP]]
158+
; CHECK-NEXT: br label [[LOOP]]
159+
;
160+
entry:
161+
br label %loop
162+
163+
loop:
164+
%index = phi i64 [ 0, %entry ], [ %index.next, %loop ]
165+
%step.add = add nuw i64 %c1, %index
166+
call void @use(i64 %step.add)
167+
%c2.plus.2 = add nuw i64 %c2, 2
168+
%index.next = add nuw i64 %step.add, %c2.plus.2
169+
br label %loop
170+
}
171+
145172
; Hoist MUL and drop NUW even if both ops have it.
146173
define void @mul_nuw(i64 %c1, i64 %c2) {
147174
; CHECK-LABEL: @mul_nuw(
@@ -241,6 +268,33 @@ loop:
241268
br label %loop
242269
}
243270

271+
; Hoist MUL and drop NUW even if both ops have it.
272+
; A version where the LHS and RHS of the outer BinOp are BinOps.
273+
define void @mul_nuw_twobinops(i64 %c1, i64 %c2) {
274+
; CHECK-LABEL: @mul_nuw_twobinops(
275+
; CHECK-NEXT: entry:
276+
; CHECK-NEXT: [[C2_PLUS_2:%.*]] = add nuw i64 [[C2:%.*]], 2
277+
; CHECK-NEXT: [[INVARIANT_OP:%.*]] = mul i64 [[C1:%.*]], [[C2_PLUS_2]]
278+
; CHECK-NEXT: br label [[LOOP:%.*]]
279+
; CHECK: loop:
280+
; CHECK-NEXT: [[INDEX:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[INDEX_NEXT_REASS:%.*]], [[LOOP]] ]
281+
; CHECK-NEXT: [[STEP_ADD:%.*]] = mul nuw i64 [[C1]], [[INDEX]]
282+
; CHECK-NEXT: call void @use(i64 [[STEP_ADD]])
283+
; CHECK-NEXT: [[INDEX_NEXT_REASS]] = mul i64 [[INDEX]], [[INVARIANT_OP]]
284+
; CHECK-NEXT: br label [[LOOP]]
285+
;
286+
entry:
287+
br label %loop
288+
289+
loop:
290+
%index = phi i64 [ 0, %entry ], [ %index.next, %loop ]
291+
%step.add = mul nuw i64 %c1, %index
292+
call void @use(i64 %step.add)
293+
%c2.plus.2 = add nuw i64 %c2, 2
294+
%index.next = mul nuw i64 %step.add, %c2.plus.2
295+
br label %loop
296+
}
297+
244298
; Hoist ADD but don't copy NUW if only one op has it.
245299
define void @add_no_nuw(i64 %c1, i64 %c2) {
246300
; CHECK-LABEL: @add_no_nuw(

0 commit comments

Comments
 (0)