[PATCH] D94429: AMDGPU: Move handling of allocation of fixed ABI inputs

Madhur Amilkanthwar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 12 10:03:22 PST 2021


madhur13490 added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp:67
+    if (UseFixedABI && CC != CallingConv::AMDGPU_Gfx)
+      ArgInfo = AMDGPUArgumentUsageInfo::FixedABIFunctionInfo;
+
----------------
arsenm wrote:
> madhur13490 wrote:
> > The constructor needs to be improved. `ArgInfo` will now hold allocated registers but the code ahead (line 93-99) is setting bools to true which is not useful and logically a dead code. Ideally, we should just do the necessary allocation in fixedABI and return.
> It's not logically dead. Whether the argument is used/needed and whether it assigned are different things. Fixed ABI is just setting the positions, and not doing the allocation. No allocation can really occur here since that's the calling convention lowering's responsibility
Well, I meant that for fixed function ABI, we surely know that certain bools are going to be true, so why not set them, set the positions and return? Something like,

 
```
 if (UseFixedABI && CC != ...) {
   PrivateSegmentBuffer = true;
   DispatchId = true;
   ImplicitArgPtr = true;
   ...
   ArgInfo = AMDGPUArgumentUsageInfo::FixedABIFunctionInfo;
   return;
 }


```


  
  


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

https://reviews.llvm.org/D94429



More information about the llvm-commits mailing list