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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:1890
+  } else
+    Arg = allocateSGPR32InputImpl(CCInfo, &AMDGPU::SGPR_32RegClass, 32);
 }
----------------
madhur13490 wrote:
> Why do we HAVE to allocate?
Otherwise another argument may end up allocated to the same register


================
Comment at: llvm/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp:67
+    if (UseFixedABI && CC != CallingConv::AMDGPU_Gfx)
+      ArgInfo = AMDGPUArgumentUsageInfo::FixedABIFunctionInfo;
+
----------------
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


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

https://reviews.llvm.org/D94429