[PATCH] D47509: [AMDGPU] Track occupancy in MFI

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 30 03:07:21 PDT 2018


arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:4258-4262
+    SDValue Res = AMDGPUTargetLowering::LowerGlobalAddress(MFI, Op, DAG);
+    MachineFunction &MF = DAG.getMachineFunction();
+    SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
+    MFI->limitOccupancy(MF);
+    return Res;
----------------
rampitec wrote:
> rampitec wrote:
> > arsenm wrote:
> > > Why is this here?
> > allocateLDSGlobal() belongs to AMDGPUMachineFunction, not SIMachineFunctionInfo. The choice is to either make it virtual or handle in the single place it is really used.
> In fact the other choice is to create SIMachineFunctionInfo::allocateLDSGlobal() which will call AMDGPUMachineFunction::allocateLDSGlobal() and then limitOccupancy(), then call it from here. Same thing basically, but more obscure to my taste. The only sound solution is to virtualize MFI, but I am not really sure it is worth it.
I don't see a reason to update this during lowering. We don't do anything with occupancy information in the DAG? Why can't you just adjust this once after the function is selected and the LDS size is known?


https://reviews.llvm.org/D47509





More information about the llvm-commits mailing list