[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:691
     DEBUG(dbgs() << " alloca is not a candidate for vectorization.\n");
-    return;
+    return true;
   }
----------------
cfang wrote:
> arsenm wrote:
> > Why is this true? It failed
> This debug message is a misleading! if tryPromoteAllocaToVector return true, it means alloca has already been vectoized, and we should return true here. 
> 
> On the other hand, if tryPromoteAllocaToVector  return false, we should continue to try to promote alloca to LDS. I will update the debug message! 
So I completely remove this debug message here at the caller site because for every possible case in tryPromoteAllocaToVector,
a debug message was dumped.


================
Comment at: test/CodeGen/AMDGPU/vector-alloca.ll:149-153
+; EG: MOV
+; EG: MOV
+; EG: MOV
+; EG: MOV
+; EG: MOVA_INT
----------------
cfang wrote:
> arsenm wrote:
> > cfang wrote:
> > > 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?
> > You had EG checks before
> I know. That's what I copied and pasted from a previous test.  
> But the purpose of this new test is to verify that even though we have a local argument, we can still promote alloca to vector.
>  So, I think a OPT checking is enough! Do ISA checking is kind of redundant. You are right, if we do ISA checking, we should do GCN checking.  
Let me know if you still went to check GCN for this new test, Thanks.


https://reviews.llvm.org/D33139