Skip to content

Commit 839abdb

Browse files
authored
[MachineLICM] Fix incorrect CSE on hoisted const load (#73007)
When hoisting an invariant load, we should not combine it with an existing load through common subexpression elimination (CSE). This is because there might be memory-changing instructions between the existing load and the end of the block entering the loop. Fixes #72855
1 parent ae10baf commit 839abdb

File tree

2 files changed

+58
-0
lines changed

2 files changed

+58
-0
lines changed

llvm/lib/CodeGen/MachineLICM.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,11 @@ bool MachineLICMBase::EliminateCSE(
14221422
if (MI->isImplicitDef())
14231423
return false;
14241424

1425+
// Do not CSE normal loads because between them could be store instructions
1426+
// that change the loaded value
1427+
if (MI->mayLoad() && !MI->isDereferenceableInvariantLoad())
1428+
return false;
1429+
14251430
if (MachineInstr *Dup = LookForDuplicate(MI, CI->second)) {
14261431
LLVM_DEBUG(dbgs() << "CSEing " << *MI << " with " << *Dup);
14271432

@@ -1475,6 +1480,9 @@ bool MachineLICMBase::EliminateCSE(
14751480
/// Return true if the given instruction will be CSE'd if it's hoisted out of
14761481
/// the loop.
14771482
bool MachineLICMBase::MayCSE(MachineInstr *MI) {
1483+
if (MI->mayLoad() && !MI->isDereferenceableInvariantLoad())
1484+
return false;
1485+
14781486
unsigned Opcode = MI->getOpcode();
14791487
for (auto &Map : CSEMap) {
14801488
// Check this CSEMap's preheader dominates MI's basic block.

llvm/test/CodeGen/AArch64/machine-licm-hoist-load.ll

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,56 @@ for.exit: ; preds = %for.body
448448
ret i64 %spec.select
449449
}
450450

451+
; See issue https://github.com/llvm/llvm-project/issues/72855
452+
;
453+
; When hoisting instruction out of the loop, ensure that loads are not common
454+
; subexpressions eliminated. In this example pointer %c may alias pointer %b,
455+
; so when hoisting `%y = load i64, ptr %b` instruction we can't replace it with
456+
; `%b.val = load i64, ptr %b`
457+
;
458+
define i64 @hoisting_no_cse(ptr %a, ptr %b, ptr %c, i64 %N) {
459+
; CHECK-LABEL: hoisting_no_cse:
460+
; CHECK: // %bb.0: // %entry
461+
; CHECK-NEXT: ldr x8, [x1]
462+
; CHECK-NEXT: add x8, x8, #1
463+
; CHECK-NEXT: str x8, [x2]
464+
; CHECK-NEXT: mov x8, xzr
465+
; CHECK-NEXT: ldr x9, [x1]
466+
; CHECK-NEXT: .LBB7_1: // %for.body
467+
; CHECK-NEXT: // =>This Inner Loop Header: Depth=1
468+
; CHECK-NEXT: ldr x10, [x0], #8
469+
; CHECK-NEXT: ldr x10, [x10]
470+
; CHECK-NEXT: cmp x10, x9
471+
; CHECK-NEXT: cinc x8, x8, eq
472+
; CHECK-NEXT: subs x3, x3, #1
473+
; CHECK-NEXT: b.ne .LBB7_1
474+
; CHECK-NEXT: // %bb.2: // %for.exit
475+
; CHECK-NEXT: mov x0, x8
476+
; CHECK-NEXT: ret
477+
entry:
478+
%b.val = load i64, ptr %b
479+
%b.val.changed = add i64 %b.val, 1
480+
store i64 %b.val.changed, ptr %c
481+
br label %for.body
482+
483+
for.body: ; preds = %entry, %for.body
484+
%idx = phi i64 [ %inc, %for.body ], [ 0, %entry ]
485+
%sum = phi i64 [ %spec.select, %for.body ], [ 0, %entry ]
486+
%arrayidx = getelementptr inbounds ptr, ptr %a, i64 %idx
487+
%0 = load ptr, ptr %arrayidx, align 8
488+
%x = load i64, ptr %0
489+
%y = load i64, ptr %b
490+
%cmp = icmp eq i64 %x, %y
491+
%add = zext i1 %cmp to i64
492+
%spec.select = add i64 %sum, %add
493+
%inc = add nuw i64 %idx, 1
494+
%exitcond = icmp eq i64 %inc, %N
495+
br i1 %exitcond, label %for.exit, label %for.body
496+
497+
for.exit: ; preds = %for.body
498+
ret i64 %spec.select
499+
}
500+
451501
declare i32 @bcmp(ptr, ptr, i64)
452502
declare i32 @memcmp(ptr, ptr, i64)
453503
declare void @func()

0 commit comments

Comments
 (0)