[PATCH] D129686: [RISCV] Reuse a materialised global address in preference to merging into a load/store

[Alex Bradbury via Phabricator via llvm-commits]

asb created this revision.
asb added reviewers: reames, craig.topper, luismarques.
Herald added subscribers: wingo, sunshaoce, pmatos, VincentWu, luke957, StephenFan, vkmr, frasercrmck, evandro, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

This isn't ready for full review - I need to clean up and add in more tests that reflect issues found when working on this. But posting for any initial feedback from other timezones ahead of me looking again tomorrow (UK time).

I noted in D129178 <https://reviews.llvm.org/D129178> that in some cases, code sequences like:

  lui a1, %hi(.L_MergedGlobals)
  sw a0, %lo(.L_MergedGlobals)(a1)
  addi a1, a1, %lo(.L_MergedGlobals)
  ... (other users of a1)

Where altering the `sw` to use the global address once it's fully materialised into `a1` might be beneficial for code size (increasing the chance the `sw` is compressible). Such code patterns can exist without globals merging, but the globals merging code makes them much more common.

This patch achieves this by:

- Altering `SelectAddrRegImm` so it won't fold in an ADD_LO if it has users that aren't memory operations (which is typically the case when other offsets of the global are calculated with ADDs, which normally results in the global address being materialised into a register)
- Adding a peephole for handling the case where this turned out to be a bad idea - which can happen if all of those global offset calculations were merged into memory operations. Given the work Craig has done to remove the store/addi peephole, this isn't ideal...

Any initial thoughts very welcome. Arguably this should be gated on the C extension being enabled (unless we see reducing the number of relocations as worthwhile in and of itself).


https://reviews.llvm.org/D129686

Files:
  llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
  llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
  llvm/test/CodeGen/RISCV/byval.ll
  llvm/test/CodeGen/RISCV/callee-saved-fpr32s.ll
  llvm/test/CodeGen/RISCV/callee-saved-fpr64s.ll
  llvm/test/CodeGen/RISCV/callee-saved-gprs.ll
  llvm/test/CodeGen/RISCV/double-mem.ll
  llvm/test/CodeGen/RISCV/float-mem.ll
  llvm/test/CodeGen/RISCV/fold-addi-loadstore.ll
  llvm/test/CodeGen/RISCV/global-merge-minsize.ll
  llvm/test/CodeGen/RISCV/global-merge-offset.ll
  llvm/test/CodeGen/RISCV/global-merge.ll
  llvm/test/CodeGen/RISCV/half-mem.ll
  llvm/test/CodeGen/RISCV/mem.ll
  llvm/test/CodeGen/RISCV/mem64.ll
  llvm/test/CodeGen/RISCV/memcpy-inline.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-store-merge-crash.ll
  llvm/test/CodeGen/RISCV/zext-with-load-is-free.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129686.444379.patch
Type: text/x-patch
Size: 97785 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220713/dbedcb19/attachment.bin>