Please review fixes for llvm PR 15840

Tijl Coosemans tijl at freebsd.org
Fri Sep 6 10:02:40 PDT 2013


Hi Tim,

On Fri, 6 Sep 2013 15:07:04 +0100 Tim Northover 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 wrote the original patch but unfortunately I cannot really explain
what's causing the bug.  I'm completely new to LLVM development but I
worked out a patch anyway because nobody else seemed to be picking up
this bug report and it's preventing clang from building gstreamer which
over a thousand other packages depend on including most desktop
environments so it's bit of a blocker to shipping FreeBSD 10 without gcc.

As I understand it the original code created a MachineSDNode during DAG
lowering but the documentation for MachineSDNode says: "These nodes are
created during the instruction selection proper phase."
http://llvm.org/docs/doxygen/html/classllvm_1_1MachineSDNode.html

So something between lowering and instruction selection is getting
confused about there being a MachineSDNode but I cannot tell you what.
Or maybe this node just isn't set up properly?  In any case the patch
simply delays creation of the MachineSDNode until instruction selection.
This approach can be expanded by turning the zero in the instruction into
a separate node during lowering (add DAG.getConstant(0, MVT::i32) as an
operand to the X86ISD::FENCE node) and then during instruction selection
use X86::LOCK_OR32mr instead of X86::LOCK_OR32mi8 if a register happens
to hold a zero.  I don't know what to put in X86InstrInfo.td to do this
though.

Anyway, thanks for looking into this and hopefully this helps you to find
the real cause of the bug or if not, maybe you know who else Dimitry and
I could talk to?



More information about the llvm-commits mailing list