[PATCH] D94648: [amdgpu] Implement lower function LDS pass

[Mahesha S via Phabricator via llvm-commits]

hsmhsm added a comment.

In D94648#2769780 <https://reviews.llvm.org/D94648#2769780>, @JonChesterfield wrote:

> In D94648#2758386 <https://reviews.llvm.org/D94648#2758386>, @arsenm wrote:
>
>> In D94648#2757793 <https://reviews.llvm.org/D94648#2757793>, @JonChesterfield wrote:
>>
>>> 
>>
>>
>>
>>> edit: lowerFormalArguments is not called if there are no formal arguments to the kernel. Test case I started from does pass arguments to the kernel, but they were unused and eliminated.
>>
>> Not seeing how it isn't called with no arguments? It should still be called anyway
>
> I expected that too, but debug statements around allocateLDSGlobal didn't fire, and looking at the control flow around the sdag and globalisel paths lowerFormalArguments is called from within a loop `for (const Argument &Arg : F.args()) {}`. I may be missing something of course (didn't put a lot of time into chasing this), but it definitely looks like lowerFormalArguments doesn't get called when there are no arguments.

Hi Jon,

For the 0 argument kernel, did you put debug statement within SITargetLowering::LowerFormalArguments() and test whether it will hit or not?  My experimentation shows that it is indeed hit for 0 arg kernels too. So it is not problem with 0 arg kernel.

Problem is within the function - AMDGPUMachineFunction::allocateModuleLDSGlobal() that you wrote.

The statement

  GlobalVariable *GV = M->getGlobalVariable("llvm.amdgcn.module.lds");

is suppose to be replaced by

  GlobalVariable *GV = M->getGlobalVariable("llvm.amdgcn.module.lds", true);

My understanding is - For Module to search for a internal global variable successfully within the symbol table, we need to explicitly tell it to do so by passing "true" as an additional arg to getGlobalVariable(). Otherwise, the internal symbol won't be find.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94648