This is an archive of the discontinued LLVM Phabricator instance.

Encode alignment attribute for `atomicrmw`
ClosedPublic

Authored by gchatelet on Jul 9 2020, 1:29 AM.

Details

Summary

This is a follow up patch to D83136 adding the align attribute to atomicwmw.

Diff Detail

Event Timeline

gchatelet created this revision.Jul 9 2020, 1:29 AM
jfb added inline comments.Jul 9 2020, 8:23 AM
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
5139

I think you want this instead:

MaybeAlign Align;
if (Error Err = parseAlignmentValue(Record[6], Align))
  return Err;

?

llvm/test/Bitcode/compatibility.ll
756

I think you still need tests *without* alignment?

jyknight added inline comments.Jul 29 2020, 3:38 PM
llvm/docs/LangRef.rst
9538

It's worth explicitly noting here that this default alignment assumption is different than the alignment used for the load/store instructions when align isn't specified.

gchatelet updated this revision to Diff 290818.Sep 9 2020, 1:19 PM
gchatelet marked 3 inline comments as done.

rebase

llvm/lib/Bitcode/Reader/BitcodeReader.cpp
5139

Absolutely!

gchatelet updated this revision to Diff 290915.Sep 10 2020, 2:39 AM
  • Use non trivial alignment in tests
jyknight accepted this revision.Feb 11 2021, 11:40 AM

This seems to have fallen off everyone's radar -- sorry about that!

The change looks good to me -- and since it's prerequisite to another change I'm making, I'm gonna go ahead and rebase+push it.

This revision is now accepted and ready to land.Feb 11 2021, 11:40 AM
This revision was landed with ongoing or failed builds.Feb 11 2021, 12:18 PM
This revision was automatically updated to reflect the committed changes.

Thx @jyknight ! Let me know if you prefer me to push it ( and D87443 ).
If you do push them please mention the two reviews (D87443 and D83465) in the description so we can track everything and abandon these changes.

Thx @jyknight ! Let me know if you prefer me to push it ( and D87443 ).
If you do push them please mention the two reviews (D87443 and D83465) in the description so we can track everything and abandon these changes.

That was fast ^__^