This is an archive of the discontinued LLVM Phabricator instance.

Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg.
ClosedPublic

Authored by jyknight on Feb 22 2021, 1:29 PM.

Details

Summary

And then push those change throughout LLVM.

Keep the old signature in Clang's CGBuilder for now -- that will be
updated in a follow-on patch.

TODO: what about MLIR?

Diff Detail

Event Timeline

jyknight created this revision.Feb 22 2021, 1:29 PM
jyknight requested review of this revision.Feb 22 2021, 1:29 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptFeb 22 2021, 1:29 PM

Looks like you didn't update the .bc reader/writer and the .ll printer/parser.

Looks like you didn't update the .bc reader/writer and the .ll printer/parser.

The instructions already had alignment support added to the type constructor and IR/BC in prior changes by gchatelet -- this is just updating the IRBuilder constructor.

gchatelet added inline comments.Feb 23 2021, 8:10 AM
llvm/lib/CodeGen/AtomicExpandPass.cpp
1037

Add an error message?

Looks like you didn't update the .bc reader/writer and the .ll printer/parser.

The instructions already had alignment support added to the type constructor and IR/BC in prior changes by gchatelet -- this is just updating the IRBuilder constructor.

For the record the previous patches introducing the alignment were D87443 and D83465.

rjmccall accepted this revision.Feb 23 2021, 2:09 PM

Ah, sorry, I missed that. In that case, LGTM, but you may want to respond to other reviews.

This revision is now accepted and ready to land.Feb 23 2021, 2:09 PM
gchatelet accepted this revision.Feb 24 2021, 12:07 AM

LGTM as well

This revision was landed with ongoing or failed builds.Feb 25 2021, 3:30 PM
This revision was automatically updated to reflect the committed changes.
jrtc27 added a subscriber: jrtc27.Feb 25 2021, 3:42 PM
jrtc27 added inline comments.
llvm/lib/CodeGen/AtomicExpandPass.cpp
1037

Doesn't this introduce another copy of the same problem that D91256 is fixing (even if that one happens to also be fixed by this patch)?

jyknight added inline comments.Feb 26 2021, 8:08 AM
llvm/lib/CodeGen/AtomicExpandPass.cpp
1037

It sure does, although in a different function. :) Fixed in 740e69b6fdc2b1415065f3d20acc4d10a73edb00, thanks.