This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix for negative offsets in buffer/tbuffer intrinsics
ClosedPublic

Authored by tpr on Sep 29 2018, 1:04 AM.

Details

Summary

The new buffer/tbuffer intrinsics handle an out-of-range immediate
offset by moving/adding offset&-4096 to a vgpr, leaving an in-range
immediate offset, with a chance of the move/add being CSEd for similar
loads/stores.

However it turns out that a negative offset in a vgpr is illegal, even
if adding the immediate offset makes it legal again.

Therefore, this commit disables the offset&-4096 thing if the offset is
negative.

Change-Id: Ie02f0a74f240a138dc2a29d17cfbd9e350e4ed13

Diff Detail

Event Timeline

tpr created this revision.Sep 29 2018, 1:04 AM
mareko accepted this revision.Sep 29 2018, 5:07 PM
This revision is now accepted and ready to land.Sep 29 2018, 5:07 PM
arsenm added a comment.Oct 1 2018, 8:24 PM

Isn't there a sign bit is known zero check for these somewhere already?

tpr added a comment.Oct 3 2018, 2:58 AM

For the old intrinsics, there used to be code called from dag pattern matching to convert a single offset into some combination of voffset, soffset and instoffset. Is that what you're referring to?

I replaced that with this code here, earlier on in ISelLowering when converting the intrinsic to a DAG node, that converts an offset into some combination of voffset and instoffset.

This revision was automatically updated to reflect the committed changes.