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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 3 13:01:54 PST 2023


arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:666
+    LLVMContext &Ctx = M->getContext();
+    auto Type = M->getDataLayout().getIntPtrType(Ctx, AMDGPUAS::LOCAL_ADDRESS);
+    GV->setMetadata(LLVMContext::MD_absolute_symbol,
----------------
Don't like variables shadowing type names (also hate the LLVM capitalized naming convention). auto *IntTy?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:103
+AMDGPUMachineFunction::parseSingleOperandMetadata(MDNode *MD) {
+  if (MD && MD->getNumOperands() == 1) {
+    if (ConstantInt *KnownSize =
----------------
Early return (or just move the null check to the caller). 

The verifier should really be enforcing the operand count so every user doesn't need to check that it's 1. Should add the verifier check and drop the check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144221



More information about the llvm-commits mailing list