[PATCH] D144221: [amdgpu][nfc] Replace ad hoc LDS frame recalculation with absolute_symbol MD

Sat Feb 25 02:35:16 PST 2023

JonChesterfield added a comment.

This is still a NFC in terms of machine code but introducing the metadata during LDS lowering introduces a lot of churn into the tests, leaving updating those for a later patch.

Strategy of emitting metadata in the IR pass and then checking allocation matches it removes the frame calculation functions from AMDGPUMachineFunction and reduces (but doesn't quite eliminate) the function/struct symbol name string effort. It's probably possible to tag functions with the struct directly (using more metadata) to remove that entirely.

The primary drawback is that this uses metadata to convey information where we will miscompile if that metadata is removed, directly at odds with the design intent of metadata. However I claim that ship has sailed, and we already assume that some metadata makes it to the backend safely. That the lowering pass runs as part of codegen, not as part of per-TU opt, reduces the risk of another pass invalidating it.

Aside from the metadata-might-be-invalidated hazard, I consider this approach better than the frame calculation in every respect, and would have done it this way around originally if I'd noticed the absolute symbol construct before writing the frame calculation approach.

This is ready for code review - hopefully nothing too surprising in the implementation - but needs a large mechanical update to eight tests to introduce the metadata and change some integers.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:111
 }

+std::optional<uint32_t>
----------------
Function has four call sites across three files, think it's probably worth factoring out.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144221/new/

https://reviews.llvm.org/D144221