Hi,
I have done a refactoring that merges AtomicExpandLoadLinkedPass and
X86AtomicExpandPass, fixing the bug above in the process. I am wondering on
what is the best way to cut such a patch in pieces for review. I have
attached the corresponding patches; they are not completely ready for
review (mostly missing tests), I would just like to make sure that the
general approach seems reasonable.
If so, I will finish them and send them on Phabricator for review.
Thanks,
Robin
On Fri, Aug 15, 2014 at 2:26 AM, Tim Northover <t.p.northover at
gmail.com>
wrote:
> > From my reading of Atomics.rst, it would be sound to reorder (It does
not
> > say much about load-linked, so I am treating it as a normal load here)
> >
> >> store seq_cst
> >> fence release
> >> load-linked monotonic
> >
> > into
> >
> >> load-linked monotonic
> >> store seq_cst
> >> fence release
>
> > Which would make an execution ending in %old_x = %old_y = 0 possible,
> while
> > it is impossible in the original program.
>
> Hmm, evil. Well, I'm convinced. Thanks very much for taking the time
> to come up with an example and telling us about it.
>
> > Fixing it is a two line change in insertLeadingFence, but it triggers
> some
> > test failures, both because of tests looking specifically for a fence
> > release here,
>
> That's fine, we can change those easily enough. And the "dmb
ishst"
> (as I understand it, it *is* a release fence, but not almost certainly
> not suitable for preventing this reordering).
>
> Cheers.
>
> Tim.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140815/ab02f8a5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-hooks-to-TargetLowering-for-a-future-AtomicExpan.patch
Type: text/x-patch
Size: 5511 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140815/ab02f8a5/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Copy-AtomicExpandLoadLinkedPass-X86AtomicExpandPass-.patch
Type: text/x-patch
Size: 21489 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140815/ab02f8a5/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-X86-Use-AtomicExpandPass-instead-of-X86AtomicExpandP.patch
Type: text/x-patch
Size: 5931 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140815/ab02f8a5/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Erase-X86AtomicExpandPass.patch
Type: text/x-patch
Size: 12605 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140815/ab02f8a5/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-ARM-Use-AtomicExpandPass-instead-of-AtomicExpandLoad.patch
Type: text/x-patch
Size: 66115 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140815/ab02f8a5/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-AArch64-Use-AtomicExpandPass-instead-of-AtomicExpand.patch
Type: text/x-patch
Size: 4807 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140815/ab02f8a5/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-Erase-AtomicExpandLoadLinkedPass.patch
Type: text/x-patch
Size: 20517 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140815/ab02f8a5/attachment-0006.bin>