[PATCH] D33139: AMDGPU/SI: Move the local memory usage related checking after calling convention checking in PromoteAlloca

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:604
+  CurrentLocalMemUsage = 0;
+  for (GlobalVariable &GV : Mod->globals()) {
+    if (GV.getType()->getAddressSpace() != AS.LOCAL_ADDRESS)
----------------
There's a possible hazard with aliases of globals but I think that's been an existing problem


================
Comment at: lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:608-609
+
+    for (const User *U : GV.users()) {
+      const Instruction *Use = dyn_cast<Instruction>(U);
+      if (!Use)
----------------
The user could be a constant expression which is transitively used by an instruction but I guess you're just moving this


================
Comment at: lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:705-706
 
+  if (!hasSufficientLocalMem(ContainingFunction))
+    return false;
+
----------------
This is being called for every single alloca in the function. This should probably be checked once earlier


================
Comment at: test/CodeGen/AMDGPU/vector-alloca.ll:149-153
+; EG: MOV
+; EG: MOV
+; EG: MOV
+; EG: MOV
+; EG: MOVA_INT
----------------
Should use the GCN instruction checks


https://reviews.llvm.org/D33139