Skip to content

Commit 8838874

Browse files
committed
[SimplifyCFG] Check alignment when speculating stores
When speculating a store based on a preceding load/store, we need to ensure that the speculated store does not have a higher alignment (which might only be guaranteed by the branch condition). There are various ways in which this could be strengthened (we could get or enforce the alignment), but for now just do the simple check against the preceding load/store. Fixes #89672.
1 parent b64e483 commit 8838874

File tree

2 files changed

+13
-8
lines changed

2 files changed

+13
-8
lines changed

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2888,15 +2888,16 @@ static Value *isSafeToSpeculateStore(Instruction *I, BasicBlock *BrBB,
28882888
// simple, to avoid introducing a spurious non-atomic write after an
28892889
// atomic write.
28902890
if (SI->getPointerOperand() == StorePtr &&
2891-
SI->getValueOperand()->getType() == StoreTy && SI->isSimple())
2891+
SI->getValueOperand()->getType() == StoreTy && SI->isSimple() &&
2892+
SI->getAlign() >= StoreToHoist->getAlign())
28922893
// Found the previous store, return its value operand.
28932894
return SI->getValueOperand();
28942895
return nullptr; // Unknown store.
28952896
}
28962897

28972898
if (auto *LI = dyn_cast<LoadInst>(&CurI)) {
28982899
if (LI->getPointerOperand() == StorePtr && LI->getType() == StoreTy &&
2899-
LI->isSimple()) {
2900+
LI->isSimple() && LI->getAlign() >= StoreToHoist->getAlign()) {
29002901
// Local objects (created by an `alloca` instruction) are always
29012902
// writable, so once we are past a read from a location it is valid to
29022903
// also write to that same location.

llvm/test/Transforms/SimplifyCFG/speculate-store.ll

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -240,14 +240,16 @@ if.end:
240240
ret i32 %add
241241
}
242242

243-
; FIXME: This is a miscompile.
244243
define void @wrong_align_store(ptr %A, i32 %B, i32 %C, i32 %D) {
245244
; CHECK-LABEL: @wrong_align_store(
246245
; CHECK-NEXT: entry:
247246
; CHECK-NEXT: store i32 [[B:%.*]], ptr [[A:%.*]], align 4
248247
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 [[D:%.*]], 42
249-
; CHECK-NEXT: [[SPEC_STORE_SELECT:%.*]] = select i1 [[CMP]], i32 [[C:%.*]], i32 [[B]]
250-
; CHECK-NEXT: store i32 [[SPEC_STORE_SELECT]], ptr [[A]], align 8
248+
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[RET_END:%.*]]
249+
; CHECK: if.then:
250+
; CHECK-NEXT: store i32 [[C:%.*]], ptr [[A]], align 8
251+
; CHECK-NEXT: br label [[RET_END]]
252+
; CHECK: ret.end:
251253
; CHECK-NEXT: ret void
252254
;
253255
entry:
@@ -263,15 +265,17 @@ ret.end:
263265
ret void
264266
}
265267

266-
; FIXME: This is a miscompile.
267268
define void @wrong_align_load(i32 %C, i32 %D) {
268269
; CHECK-LABEL: @wrong_align_load(
269270
; CHECK-NEXT: entry:
270271
; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
271272
; CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[A]], align 4
272273
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 [[D:%.*]], 42
273-
; CHECK-NEXT: [[SPEC_STORE_SELECT:%.*]] = select i1 [[CMP]], i32 [[C:%.*]], i32 [[TMP0]]
274-
; CHECK-NEXT: store i32 [[SPEC_STORE_SELECT]], ptr [[A]], align 8
274+
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[RET_END:%.*]]
275+
; CHECK: if.then:
276+
; CHECK-NEXT: store i32 [[C:%.*]], ptr [[A]], align 8
277+
; CHECK-NEXT: br label [[RET_END]]
278+
; CHECK: ret.end:
275279
; CHECK-NEXT: ret void
276280
;
277281
entry:

0 commit comments

Comments
 (0)