| Issue |
89060
|
| Summary |
MemOperands not updated during ISel argument copy elision, may lead to miscompile
|
| Labels |
new issue
|
| Assignees |
|
| Reporter |
mikaelholmen
|
Reproduce with: 06eedffe0d27
```
llc bbi-94617_3.ll -o - -debug
```
[bbi-94617_3.ll.gz](https://github.com/llvm/llvm-project/files/15010509/bbi-94617_3.ll.gz)
If we look at the debug printouts, in "Initial selection DAG:" we get
```
SelectionDAG has 29 nodes:
t0: ch,glue = EntryToken
t2: i32,ch = CopyFromReg t0, Register:i32 %0
t3: i16 = truncate t2
t5: i32,ch = CopyFromReg t0, Register:i32 %1
t6: i16 = truncate t5
t8: i32,ch = CopyFromReg t0, Register:i32 %2
t9: i16 = truncate t8
t11: i32,ch = CopyFromReg t0, Register:i32 %3
t12: i16 = truncate t11
t14: i32,ch = CopyFromReg t0, Register:i32 %4
t15: i16 = truncate t14
t17: i32,ch = CopyFromReg t0, Register:i32 %5
t18: i16 = truncate t17
t21: i16,ch = load<(load (s16) from %fixed-stack.0)> t0, FrameIndex:i64<-1>, undef:i64
t23: i64 = Constant<0>
t24: ch = store<(store (s16) into @v_781, align 1)> t21:1, t21, GlobalAddress:i64<ptr @v_781> 0, undef:i64
t26: ch = store<(store (s16) into %ir.param.addr, align 1)> t24, Constant:i16<666>, FrameIndex:i64<-1>, undef:i64
t28: ch = X86ISD::RET_GLUE t26, TargetConstant:i32<0>
```
Especially consider
```
t21: i16,ch = load<(load (s16) from %fixed-stack.0)> t0, FrameIndex:i64<-1>, undef:i64
```
and
```
t26: ch = store<(store (s16) into %ir.param.addr, align 1)> t24, Constant:i16<666>, FrameIndex:i64<-1>, undef:i64
```
The load and store both access "FrameIndex:i64<-1>", but the MemOperands look like
```
load (s16) from %fixed-stack.0
```
and
```
store (s16) into %ir.param.addr, align 1
```
so they access exactly the same memory on the stack, but according to the MemOperands it looks like they access different objects? I think this is wrong.
If we look further at the debug printouts at "MI Scheduling" we get
```
SU(0): %6:gr16 = MOV16rm %fixed-stack.0, 1, $noreg, 0, $noreg :: (load (s16) from %fixed-stack.0, align 16)
# preds left : 0
# succs left : 1
# rdefs left : 0
Latency : 4
Depth : 0
Height : 4
Successors:
SU(2): Data Latency=4 Reg=%6
Single Issue : false;
[...]
SU(2): MOV16mr %7:gr64, 1, $noreg, 0, $noreg, %6:gr16 :: (store (s16) into @v_781, align 1)
# preds left : 2
# succs left : 0
# rdefs left : 0
Latency : 1
Depth : 4
Height : 0
Predecessors:
SU(1): Data Latency=4 Reg=%7
SU(0): Data Latency=4 Reg=%6
Single Issue : false;
SU(3): MOV16mi %fixed-stack.0, 1, $noreg, 0, $noreg, 666 :: (store (s16) into %ir.param.addr, align 16)
# preds left : 0
# succs left : 0
# rdefs left : 0
Latency : 1
Depth : 0
Height : 0
Single Issue : false;
```
So there is no dependency whatsoever between the load, SU(0) and the store SU(3), even if they access the same memory.
On my downstream target this then lead to a miscompile when the scheduler choose to change the order of the load and store.
I think the problem is that SelectionDAGISel::LowerArguments and tryToElideArgumentCopy does rewrites and change the FI used in some instructions, but they leave the MemOperands unchanged.
This seems to be old but it recently lead to a miscompile for my target when the load and store suddenly executed in the wrong order.
![]()
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
