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

[Changpeng Fang via Phabricator via llvm-commits]

cfang 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)
----------------
arsenm wrote:
> There's a possible hazard with aliases of globals but I think that's been an existing problem
Will take a look and fix it in a separate patch if needed!


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


================
Comment at: lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:705-706
 
+  if (!hasSufficientLocalMem(ContainingFunction))
+    return false;
+
----------------
arsenm wrote:
> This is being called for every single alloca in the function. This should probably be checked once earlier
Done! Move the check before the loop, and use an argument to handleAlloca to carry the check result. Will update the diff late. 


================
Comment at: test/CodeGen/AMDGPU/vector-alloca.ll:149-153
+; EG: MOV
+; EG: MOV
+; EG: MOV
+; EG: MOV
+; EG: MOVA_INT
----------------
arsenm wrote:
> Should use the GCN instruction checks
what is GCN instruction check? Do you mean should use llc to compile to ISA and do checking?


https://reviews.llvm.org/D33139