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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec 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;
----------------
arsenm wrote:
> 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?
Thanks! Moved to finalizeLowering().


https://reviews.llvm.org/D47509