[LLVMbugs] [Bug 8156] New: LegalizeOp incorrect node updates!

[bugzilla-daemon at llvm.org]

http://llvm.org/bugs/show_bug.cgi?id=8156

           Summary: LegalizeOp incorrect node updates!
           Product: libraries
           Version: trunk
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: Common Code Generator Code
        AssignedTo: unassignedbugs at nondot.org
        ReportedBy: bruno.cardoso at gmail.com
                CC: llvmbugs at cs.uiuc.edu


Created an attachment (id=5496)
 --> (http://llvm.org/bugs/attachment.cgi?id=5496)
"call DAG.viewGraph()" output

The current implementation of LegalizeOp has the following behavior:

Consider the custom lowering or expansion of any node in the DAG. After
a new node is returned in those operations, LegalizeOp is called for this
node again, but it doesn't update the DAG to reflect the new uses and
to remove the old ones. This leads to the condition where it's impossible
to detect some potential folds during lowering time, because some node
we're trying to fold can have more than one use, as its previous users
are still alive in the DAG when they shouldn't.

This currently blocks the rewritting of VECTOR_SHUFFLE lowering for x86,
which has several shuffles with different costs, making it important
to know during lowering time which nodes are potential folds.

Proposed solution: Rewrite the LegalizeOp to work in a work list based
fashion and to always update the DAG in the same way DAGCombiner does.

To reproduce the problem, try this example with the attached bitcode:

llc < v02.ll -march=x86 -mattr=+sse2,-sse41

A quick explanation of what happens here:

1) insertelement is expanded to a vector_shuffle<0,2>
2) x86 LowerVECTOR_SHUFFLE is called for vector_shuffle<0,2>
3) Breakpoint at the beginning of this function, and
asks for "call DAG.viewGraph()" using gdb, the resulting graph
is attached.

The DAG at this point will display to uses for the "load" node,
one coming from the old (already expanded) insertelement and one
coming from vector_shuffle<0,2>. This leads the lowering to
select X86ISD::UNPCKLPD instead of X86ISD::MOVHPS. We circuvent
that using a small hack now in x86 sse td files:

def : Pat<(v2f64 (X86Unpcklpd VR128:$src1,
                   (scalar_to_vector (loadf64 addr:$src2)))),
         (MOVHPDrm VR128:$src1, addr:$src2)>;

But this is something we obviously don't want. The same problem
also occurs for lots of other examples, and every attempt to clean
up the vector_shuffle handling right now keeps triggering more
of this problem, so a hack here and there is not sufficient anymore
to solve the issue.

-- 
Configure bugmail: http://llvm.org/bugs/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.