Page MenuHomePhabricator

AMDGPU: Add a16 feature to gfx10

Authored by arsenm on Jan 17 2020, 9:01 AM.



It seems like this was accidentally dropped, breaking all a16 image
intrinsics for gfx10. This fixes failing/asserting during
lowering. With the feature added, the selected instruction fails the
verifier. Guess at how to fix this, which seems to have confused the
dword and component counts.

Duplicate the test and generate the checks since I know very little
about how these are really supposed to operate.

Diff Detail

Event Timeline

arsenm created this revision.Jan 17 2020, 9:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 17 2020, 9:01 AM
arsenm updated this revision to Diff 238812.Jan 17 2020, 9:58 AM
arsenm edited the summary of this revision. (Show Details)
arsenm added a reviewer: rtaylor.
rampitec added inline comments.Jan 17 2020, 10:38 AM

Could you add it right to the FeatureGFX10 maybe?

sebastian-ne marked an inline comment as done.Jan 27 2020, 6:50 AM
sebastian-ne added a subscriber: sebastian-ne.

The added CodeGen/AMDGPU/llvm.amdgcn.image.a16.dim.ll fails for gfx10, but a16 does not work anyway at the moment.

5628 ↗(On Diff #238812)

Are you sure that’s right?
The test CodeGen/AMDGPU/llvm.amdgcn.image.nsa.ll fails for me with this change.

arsenm marked an inline comment as done.Jan 27 2020, 6:53 AM
arsenm added inline comments.
5628 ↗(On Diff #238812)

No, it's probably wrong

sebastian-ne added inline comments.Jan 29 2020, 6:02 AM
3679 ↗(On Diff #238812)

I think this should be the following because the extra arguments like bias are still f32 and should not be divided by two:

unsigned AddrComponents =
                     (BaseOpcode->Gradients ? Dim->NumGradients : 0) +
                     (BaseOpcode->Coordinates ? Dim->NumCoords : 0) +
                     (BaseOpcode->LodOrClampOrMip ? 1 : 0);

unsigned AddrWords = BaseOpcode->NumExtraArgs +
                     (IsA16 ? (AddrComponents + 1) / 2 : AddrComponents);
arsenm marked an inline comment as done.Jan 29 2020, 7:08 AM
arsenm added inline comments.
3679 ↗(On Diff #238812)

Feel free to commandeer, I'm probably not going to get back to fixing the DAG version anytime soon

arsenm abandoned this revision.Feb 4 2020, 2:02 PM