[PATCH] D133422: [amdgpu] Remove unnecessary removal of constantexpr, expected to fix O0 problem

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added a comment.

The current revision will fix the O0 codegen bug @carlo.bertolli is looking at.

It's possible, though awkward, to keep the original idea of deleting the call to replaceConstantUsesInFunction if the lambda passed to replaceLDSVariablesWithStruct works much harder to identify the function corresponding to a Use. That's approximately as complicated as expanding the ConstantExpr up front. Choosing this approach because it's convenient for the other WIP on LDS which replaces some expressions with function calls and thus also needs to expand ConstantExpr.

The missing test case involves a phi with constantexpr arguments from empty basic blocks, checking the intrinsic is inserted successfully. Open to alternative ideas for handling the empty basic block but would prefer to fix the current (latent) crash bug at the same time as this.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:208
+            // a no-op one to use
+            LLVMContext &Ctx = M.getContext();
+            IRBuilder<> Builder(Ctx);
----------------
This needs a test case, though the implementation that was called into by replaceConstantUsesInFunction unconditionally dereferences it which suggests empty blocks don't get here in practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133422