This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC] Incorrect parsing of flat/global atomic modifiers
ClosedPublic

Authored by dp on Dec 27 2017, 6:52 AM.

Diff Detail

Event Timeline

dp created this revision.Dec 27 2017, 6:52 AM
vpykhtin accepted this revision.Dec 27 2017, 6:57 AM

LGTM.

This revision is now accepted and ready to land.Dec 27 2017, 6:57 AM
artem.tamazov requested changes to this revision.Dec 28 2017, 5:04 AM

Please add negative tests: (a) VDST present but none GLC and (b) none VDST but GLC presents. Otherwise fine.

This revision now requires changes to proceed.Dec 28 2017, 5:04 AM
dp updated this revision to Diff 128293.Dec 28 2017, 6:22 AM

Added a few negative tests and one positive.

Note that the following code is assembled ok, though 'glc' is omitted:

flat_atomic_cmpswap v0, v[1:2], v[3:4]

This is due to the way llvm handles optional operands. In this case tablegen-generated operand matcher tries to match (expected) 'offset' to an empty list of actual operands and failed doing that. However 'offset' is an optional operand so the match is considered successful.

This would not be easy to fix, and I think we should presume this a 'feature' :-)

artem.tamazov accepted this revision.Dec 28 2017, 7:07 AM
In D41598#964823, @dp wrote:

Note that the following code is assembled ok, though 'glc' is omitted:

flat_atomic_cmpswap v0, v[1:2], v[3:4]

This would not be easy to fix, and I think we should presume this a 'feature' :-)

I think this is a low-priority bug because this does not comply to (6.2) from Bugzilla 35730. Let's update that Bugzllia but keep it open after this fix is submitted.

This revision is now accepted and ready to land.Dec 28 2017, 7:07 AM
This revision was automatically updated to reflect the committed changes.