Page MenuHomePhabricator

[AMDGPU] Support non-strictly stronger memory orderings in SIMemoryLegalizer

Authored by t-tye on Jul 23 2021, 5:09 PM.



C++20 no longer requires the failure memory ordering to be no stronger than the
success memory ordering. Adjust assert in AMD GPU SIMemoryLegalizer, and merge
instruction memory orderings

Add common operation to merge memory orders that allows non strict memory
orderings to be combined. Use it in SIMemoryLegalizer and

Diff Detail

Event Timeline

t-tye created this revision.Jul 23 2021, 5:09 PM
t-tye requested review of this revision.Jul 23 2021, 5:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2021, 5:09 PM
foad added inline comments.Jul 26 2021, 6:56 AM

You don't need any of these innermost parentheses.

foad added inline comments.Jul 26 2021, 6:58 AM

Typo "A0" for "AO".

No testcases?


Isn't this just equivalent to FailureOrdering = MMO->getFailureOrdering()?

efriedma added inline comments.Jul 26 2021, 10:34 AM

Oh, this is a loop; nevermind.

Has the IR atomic semantics been updated to match the C++20 change yet?

@arsenm See D98995, and my followups D103284/D103342. Not sure if everything is 100% fixed, but at least the basics are working.

t-tye updated this revision to Diff 361964.Jul 27 2021, 3:32 AM

Remove unnecessary ()s.

Correct comment A0->AO.

Added tests.

t-tye added a comment.Jul 27 2021, 3:34 AM

No testcases?

Added tests.




Removed innermost parenthesis.

Legalizer part LGTM. Not sure though how will it affect the rest of the languages since it implements C++20.

t-tye added a comment.Aug 7 2021, 3:46 PM

@rampitec I believe the other changes do not change the behavior from the perspective of the C++ 20 standard.

@efriedma do those changes look ok?

efriedma accepted this revision.Aug 9 2021, 12:15 PM

Haven't looked at the test changes in detail, but code changes look fine.

This revision is now accepted and ready to land.Aug 9 2021, 12:15 PM
t-tye added a comment.Aug 10 2021, 1:39 AM

Haven't looked at the test changes in detail, but code changes look fine.

Thanks @efriedma. @rampitec reviewed the other code so I will proceed to commit.

This revision was landed with ongoing or failed builds.Aug 10 2021, 1:44 AM
This revision was automatically updated to reflect the committed changes.