[PATCH] D100481: [AMDGPU] Disable forceful inline of non-kernel functions which use LDS.

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added a comment.

I agree with the intent. This would be a good change to run through the internal CI before landing.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp:125
+
+  if (!AMDGPUTargetMachine::EnableLowerModuleLDS) {
+    for (GlobalVariable &GV : M.globals()) {
----------------
What's REGION_ADDRESS? LowerModuleLDS doesn't to do anything with it so this looks like an incidental behaviour change


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:396
 bool AMDGPUTargetMachine::EnableFixedFunctionABI = false;
+bool AMDGPUTargetMachine::EnableLowerModuleLDS = false;
 
----------------
Why does the cl::opt instance initialize to true, while this initializes to false?

I'm not clear why the storage needs to become external here, but do see that the same thing has been done for the similar variables in this block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100481