This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Extend the Merge Base Offset pass to handle AUIPC+ADDI
ClosedPublic

Authored by luismarques on Apr 6 2022, 4:07 PM.

Details

Summary

Builds upon D123264, adding support to merge the low part of the LLA address into the load/store instruction offsets.

This needs more testing and more tests. But the priority is reviewing D123264, to clarify if this is even a viable approach.

Diff Detail

Event Timeline

luismarques created this revision.Apr 6 2022, 4:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 4:07 PM
luismarques requested review of this revision.Apr 6 2022, 4:07 PM
craig.topper added inline comments.Jun 23 2022, 11:55 AM
llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
402

Do we need to check that the immediate operand of the load/store is 0 before we remove it?

  • Rebase
  • Handle non-zero offsets in tail load/store instructions
luismarques marked an inline comment as done.Jun 24 2022, 4:44 PM

Nit: tweak comments.

I think calling setOffset on the pcrel_lo offset is incorrect. That always points to the auipc it shouldn't have an offset.

llvm/test/CodeGen/RISCV/fold-addi-loadstore.ll
365

This is incorrect. There shouldn't be a +4 on the pcrel_lo relocation.

378

Here too

417

And here

Fix pointing to the AUIPC label

luismarques marked 3 inline comments as done.Jul 27 2022, 2:54 AM
craig.topper added inline comments.Jul 27 2022, 10:24 AM
llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
125

This should also be skipped if Hi is AUIPC

Address review feedback

luismarques marked an inline comment as done.Jul 27 2022, 10:43 AM
This revision is now accepted and ready to land.Jul 31 2022, 10:02 PM
This revision was landed with ongoing or failed builds.Aug 1 2022, 2:30 AM
This revision was automatically updated to reflect the committed changes.