Add G16 feature for GFX10 and refactor lowerImage to be easier to read
and support 16 bit derivatives.
Details
- Reviewers
arsenm nhaehnle - Commits
- rG29a6ad94fdb9: [AMDGPU] Add G16 support to image instructions
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thank you for the fast review!
I added G16 to GlobalISel legalization. Is there a part where the image instructions get selected for GlobalISel? I didn’t find it and I guess it doesn’t pick up G16 instructions automatically.
gfx9 does not have G16 (addresses are 32 bit, derivatives 16 bit) only A16 (addresses and derivatives are 16 bit), so it’s tested in unsupported-image-g16.ll.
What is the best way to handle A16 and G16 in instruction selection for GlobalISel?
They are only encoded in the types of arguments and not in the intrinsic id. In legalization, we can simply check the type of derivative and address arguments but afterwards, all arguments are packed into a singe, large value for the non-nsa case or several 32 bit values for nsa.
I think it’s hard to detect if this was originally A16 or even only G16 with 32 bit addresses.
Should I encode this information into metadata in the legalization step and use that in instruction selection?
I think this would look like I.getMetadata("amdgpu.a16") and I.getMetadata("amdgpu.g16").
I agree that this is sub-optimal, though metadata isn't the right way to go here.
IIRC the current approach is to intuit what the original intent was based on NoReg left over by GlobalISel legalization and the like. However, now that we have G_AMDGPU_xxx MIR opcodes for image instructions after legalization, one option would be to add a magic operand to those machine instructions which encode the A16- and G16-ness.
As Nicolai suggested, I added an operand which encodes A16- and G16-ness.
A16 and G16 seem to be working now with SDag and GlobalISel.
Fixed UseNSA with A16 in GlobalISel, which decided based on the number of address components instead of dwords/registers.
LGTM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | ||
---|---|---|
5756 | Maybe state that the comment uses "hi,lo" notation? |
Maybe state that the comment uses "hi,lo" notation?