Skip to content

Commit 5e03c0a

Browse files
authored
[DAGCombiner] Fix mayAlias not accounting for scalable MMOs with offsets (#90573)
In #70452 DAGCombiner::mayAlias was taught to handle scalable sizes, but when it checks via AA->isNoAlias it didn't take into account the case where the size is scalable but there was an offset too. For the fixed length case the offset was just accounted for by adding to the LocationSize, but for the scalable case there doesn't seem to be a way to represent both a scalable and fixed part in it. So this patch works around it by bailing if there is an offset. Fixes #90559
1 parent 82219e5 commit 5e03c0a

File tree

2 files changed

+36
-9
lines changed

2 files changed

+36
-9
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28113,7 +28113,10 @@ bool DAGCombiner::mayAlias(SDNode *Op0, SDNode *Op1) const {
2811328113
#endif
2811428114

2811528115
if (UseAA && AA && MUC0.MMO->getValue() && MUC1.MMO->getValue() &&
28116-
Size0.hasValue() && Size1.hasValue()) {
28116+
Size0.hasValue() && Size1.hasValue() &&
28117+
// Can't represent a scalable size + fixed offset in LocationSize
28118+
(!Size0.isScalable() || SrcValOffset0 == 0) &&
28119+
(!Size1.isScalable() || SrcValOffset1 == 0)) {
2811728120
// Use alias analysis information.
2811828121
int64_t MinOffset = std::min(SrcValOffset0, SrcValOffset1);
2811928122
int64_t Overlap0 =

llvm/test/CodeGen/RISCV/rvv/pr90559.ll

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,43 @@
11
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
22
; RUN: llc < %s -mtriple=riscv64 -mattr=+v | FileCheck %s
33

4-
; FIXME: The i32 load and store pair isn't dead and shouldn't be omitted.
4+
; This showcases a miscompile that was fixed in #90573:
5+
; - The memset will be type-legalized to a 512 bit store + 2 x 128 bit stores.
6+
; - the load and store of q aliases the upper 128 bits store of p.
7+
; - The aliasing 128 bit store will be between the chain of the scalar
8+
; load/store:
9+
;
10+
; t54: ch = store<(store (s512) into %ir.p, align 1)> t0, ...
11+
; t51: ch = store<(store (s128) into %ir.p + 64, align 1)> t0, ...
12+
;
13+
; t44: i64,ch = load<(load (s32) from %ir.q), sext from i32> t0, ...
14+
; t50: ch = store<(store (s128) into %ir.p + 80, align 1)> t44:1, ...
15+
; t46: ch = store<(store (s32) into %ir.q), trunc to i32> t50, ...
16+
;
17+
; Previously, the scalar load/store was incorrectly combined away:
18+
;
19+
; t54: ch = store<(store (s512) into %ir.p, align 1)> t0, ...
20+
; t51: ch = store<(store (s128) into %ir.p + 64, align 1)> t0, ...
21+
;
22+
; // MISSING
23+
; t50: ch = store<(store (s128) into %ir.p + 80, align 1)> t44:1, ...
24+
; // MISSING
25+
; See also pr83017.ll: This is the same code, but relies on vscale_range instead
26+
; of -riscv-v-vector-bits-max=128.
527
define void @f(ptr %p) vscale_range(2,2) {
628
; CHECK-LABEL: f:
729
; CHECK: # %bb.0:
8-
; CHECK-NEXT: vsetvli a1, zero, e8, m4, ta, ma
9-
; CHECK-NEXT: vmv.v.i v8, 0
10-
; CHECK-NEXT: vs4r.v v8, (a0)
11-
; CHECK-NEXT: addi a1, a0, 80
30+
; CHECK-NEXT: lw a1, 84(a0)
31+
; CHECK-NEXT: addi a2, a0, 80
1232
; CHECK-NEXT: vsetivli zero, 16, e8, m1, ta, ma
1333
; CHECK-NEXT: vmv.v.i v8, 0
14-
; CHECK-NEXT: vs1r.v v8, (a1)
15-
; CHECK-NEXT: addi a0, a0, 64
16-
; CHECK-NEXT: vs1r.v v8, (a0)
34+
; CHECK-NEXT: vs1r.v v8, (a2)
35+
; CHECK-NEXT: vsetvli a2, zero, e8, m4, ta, ma
36+
; CHECK-NEXT: vmv.v.i v12, 0
37+
; CHECK-NEXT: vs4r.v v12, (a0)
38+
; CHECK-NEXT: addi a2, a0, 64
39+
; CHECK-NEXT: vs1r.v v8, (a2)
40+
; CHECK-NEXT: sw a1, 84(a0)
1741
; CHECK-NEXT: ret
1842
%q = getelementptr inbounds i8, ptr %p, i64 84
1943
%x = load i32, ptr %q

0 commit comments

Comments
 (0)