[PATCH] D117618: AMDGPU/GlobalISel: Fix flat_scratch_init handling for shaders

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp:505-509
   if (Info.hasFlatScratchInit()) {
     Register FlatScratchInitReg = Info.addFlatScratchInit(TRI);
     MF.addLiveIn(FlatScratchInitReg, &AMDGPU::SGPR_64RegClass);
     CCInfo.AllocateReg(FlatScratchInitReg);
   }
----------------
sebastian-ne wrote:
> arsenm wrote:
> > sebastian-ne wrote:
> > > Isn’t this already handled here for HSA?
> > > I didn’t hear of mesa using flat scratch (and we don’t have any flat-scratch+mesa test), so should we update the dag code instead? I wonder why we are calling `allocateHSAUserSGPRs` for non-HSA platforms in the SDAG.
> > The GISel version is factored a bit differently with a cleaner break between the kernel and non-kernel handling. allocateHSAUserSGPRs isn't called on the shader path (it's a bit ugly that it is for the DAG and it happens to work out).
> > 
> > Mesa should have some kind of ABI definition for flat scratch, but in the absence of one, we still need to produce something. This matches the DAG behavior. Alternatively I could switch mesa to follow what PAL does, but that would be a separate potentially breaking change
> I’d prefer to change the SDAG to do the same for mesa as is done for PAL.
> Duplicating the HSA init code for mesa seems wrong to me. I don’t think this would work correctly nor does the code look particularly appealing.
> From what I can see, mesa still does not use flat scratch.
I'm not attempting to address mesa's flat usage. if that change goes ahead, both would update in the future. For now gisel should just match behavior


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

https://reviews.llvm.org/D117618