[PATCH] D94648: [amdgpu] Implement lower function LDS pass
Jon Chesterfield via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 2 10:57:06 PST 2021
JonChesterfield added a comment.
@arsenm addressed last round of comments. Apologies for missing it on Friday, I don't seem to be getting email from reviews.llvm. I like the donothing hack.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerFunctionLDSPass.cpp:142
+
+ static void removeFromUsedList(Module &M, StringRef Name,
+ SmallPtrSetImpl<Constant *> &ToRemove) {
----------------
arsenm wrote:
> I feel like this should be a utility function somewhere else if it really doesn't already exist
Yes, but I don't want to propose it as such since that is likely to slow the patch process down further. Better to land it here and attempt to promote it later.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:187
+
+ static void markUsedByKernel(IRBuilder<> &Builder, Function *Func,
+ GlobalVariable *SGV) {
----------------
I quite like the donothing alternative to inline asm. It does indeed keep the use alive long enough.
A future change to the pipeline might break that, but it'll do so fairly obviously (all the openmp stuff stops working, for one). I think we go with annotated donothing for now, and implement an intrinsic -> pseudo sequence when/if it becomes necessary. Written a fairly long comment to that effect in the source.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94648/new/
https://reviews.llvm.org/D94648
More information about the llvm-commits
mailing list