This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Allow struct.buffer.*.format intrinsics to accept i32
ClosedPublic

Authored by critson on Mar 6 2020, 5:35 PM.

Details

Summary

In the same manner as struct.buffer.load / struct.buffer.store,
allow struct.buffer.load.format / struct.buffer.store.format to
return / accept any type. This simplifies front-end code gen.

Diff Detail

Event Timeline

critson created this revision.Mar 6 2020, 5:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2020, 5:35 PM
arsenm requested changes to this revision.Mar 6 2020, 8:48 PM

Needs d16 tests, and needs GlobalISel tests. I believe there was a reason format buffer intrinsics needed to be assumed to be FP, but maybe it doesn't matter

This revision now requires changes to proceed.Mar 6 2020, 8:48 PM
critson updated this revision to Diff 248897.Mar 6 2020, 9:55 PM

Add i16 (d16) tests.

Needs d16 tests, and needs GlobalISel tests. I believe there was a reason format buffer intrinsics needed to be assumed to be FP, but maybe it doesn't matter

I have added d16 tests. This already includes GlobalISel tests, unless I missed something?
I am hoping Tim Renouf (@tpr) can comment why the original intrinsics assumed FP.

tpr added a comment.Mar 10 2020, 2:22 AM

I don't know of any reason why it has to be fp. Maybe related to the prehistoric thing where isel used to need something to be fp to put it in a vgpr?

arsenm accepted this revision.Mar 10 2020, 8:53 AM

I think the important part was not changing the register layout vs. integer and FP. If these are assumed to always use the expected FP register layout in the d16 case, this is fine

This revision is now accepted and ready to land.Mar 10 2020, 8:53 AM
This revision was automatically updated to reflect the committed changes.