Please review fixes for llvm PR 15840

[Tijl Coosemans]

On Fri, 6 Sep 2013 17:06:00 +0100 Tim Northover wrote:
> On 6 September 2013 15:07, Tim Northover <t.p.northover at gmail.com> wrote:
>> 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.

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: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130920/eb473551/attachment.sig>