When pairing ldr instructions to an ldp instruction, we cannot pair two ldr
destination registers where one is a sub or super register of the other.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/test/CodeGen/AArch64/aarch64-ldst-subsuperReg-no-ldp.mir | ||
---|---|---|
30 | Is it possible to strip the IR references from the MIR test? Ideally most/all of the IR function here can be dropped. | |
187 | Are all the loads/stores and the different BBs needed for the test? Also, are the different functions needed or do they test different scenarios? |
llvm/test/CodeGen/AArch64/aarch64-ldst-subsuperReg-no-ldp.mir | ||
---|---|---|
187 | Thanks for the comment! |
llvm/test/CodeGen/AArch64/aarch64-ldst-subsuperReg-no-ldp.mir | ||
---|---|---|
30 | I spent effort trying to drop out the IR, but since some global variables such as @_MergedGlobals or @i are used in the mir, if I were to drop the IR llc would report undefined global variables error. Is there a way that I can drop the IR while still being able to run the mir? @fhahn | |
187 | Now reduced the size of the test file, resulting in only one function in the mir which exposes the LDR-to-LDP-conversion bug. |
llvm/test/CodeGen/AArch64/aarch64-ldst-subsuperReg-no-ldp.mir | ||
---|---|---|
30 | I think it should be possible to get rid of the IR references by re-writing the address computations to use the same base register but with different offsets to ensure it is clear there is no aliasing going on. Reduced test case: --- name: test tracksRegLiveness: true body: | bb.0: liveins: $x9, $x11 renamable $w10 = LDRWui renamable $x9, 2 :: (load 4) STRWui killed renamable $w10, renamable $x9, 2 :: (store 4) renamable $x10 = LDRSWui renamable $x9, 3 :: (load 4) STRWui renamable $w11, renamable $x9, 8 :: (store 4) renamable $w10 = LDRWui renamable $x9, 2 :: (load 4) RET undef $lr, implicit undef $w0 ... |
llvm/test/CodeGen/AArch64/aarch64-ldst-subsuperReg-no-ldp.mir | ||
---|---|---|
30 | @fhahn Thank you very much for this comment, now further reduced this mir test case. Also the mir test case in https://reviews.llvm.org/D86956 has been further reduced. |
LGTM, thanks!
llvm/test/CodeGen/AArch64/aarch64-ldst-subsuperReg-no-ldp.mir | ||
---|---|---|
13 | Given that the test is very short now, could you check for the full instruction sequence (LDR, LDR, RET)? Just to make sure the loads are not dropped completely (as $w10 is not really used any more)/. Also, it might eb good to add another test with the order of LDRSW/LDRW flipped. |
llvm/test/CodeGen/AArch64/aarch64-ldst-subsuperReg-no-ldp.mir | ||
---|---|---|
13 | Also made similar updates in the other patch |
llvm/test/CodeGen/AArch64/aarch64-ldst-subsuperReg-no-ldp.mir | ||
---|---|---|
13 | Thanks, that looks good! |
oh, it might be good to slightly adjust the commit message to be a bit more compact and also mention what actual bug is fixed, e.g. something like [AArch64] Avoid paring loads with same result reg.
clang-format: please reformat the code