Please review fixes for llvm PR 15840

[Tim Northover]

On 6 September 2013 15:07, Tim Northover <t.p.northover at gmail.com> wrote:
> Hi Dimitry,
>
> On 6 September 2013 11:50, Dimitry Andric <dimitry at andric.com> wrote:
>> http://llvm.org/bugs/show_bug.cgi?id=15840#c8
>> 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.

Tim.