Please review fixes for llvm PR 15840

Tijl Coosemans tijl at
Fri Sep 20 02:07:33 PDT 2013

On Fri, 6 Sep 2013 17:06:00 +0100 Tim Northover wrote:
> On 6 September 2013 15:07, Tim Northover <t.p.northover at> wrote:
>> On 6 September 2013 11:50, Dimitry Andric <dimitry at> wrote:
>>> Ping...
>> Can you explain why this is the right fix? It looks disturbingly like
>> a perturbation that just happens to bump things around enough that
>> this example works.
> I think I've worked it out myself. I've put a comment on the issue,
> but in case anyone else here has some insights:
> The underlying issue is that SelectionDAG comes across:
>     A = Node(load volatile)
>     MachineNode(ORfence)
>     B Node(load volatile)
>     Node(sub A, B)
> in chain order. When deciding whether it can merge the sub and the "A
> load", as is x86's wont, it tries to check if combining those chains
> would create a cycle.
> Unfortunately it stops looking when it reaches a MachineNode, thinking
> that everything beyond that point must be selected and hence OK
> (SelectionDAG.cpp:1863). That is an incorrect assumption in this case
> and a loopy SUB32rm is emitted.
> So the patch works by making sure there isn't a pre-selected
> MachineNode to find during this check. It would be nice to assume that
> MachineNodes only exist after selection, but if so, we've got quite a
> bit of refactoring in multiple backends to get there.
> I'll see if I can work out what the performance implications of doing
> a more thorough search here are.

Any progress on this?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <>

More information about the llvm-commits mailing list