llvm dev - Feb 2010 - [LLVMdev] SelectionDAG legality: isel creating cycles

On Monday 22 February 2010 13:26:54 David Greene wrote:
> On Monday 22 February 2010 13:06:39 Chris Lattner wrote:
> > > Just wanted to clarify in case someone was wondering about this.
> >
> > I'm currently working in this area.  What pattern is causing the
cycle?
> > Can I get a testcase?
>
> I'll see if I can generate one and file a PR.
Ah, isLegalToFold saves us on trunk.  But we lose folding due to prefetching,
which is unfortunate.

I am seeing the error with 2.5 (yes, we are upgrading!).

I guess I'll have to backport some of the isLogalToFold logic.

                                              -Dave

On Feb 22, 2010, at 12:37 PM, David Greene wrote:
> On Monday 22 February 2010 13:26:54 David Greene wrote:
>> On Monday 22 February 2010 13:06:39 Chris Lattner wrote:
>>>> Just wanted to clarify in case someone was wondering about
this.
>>> 
>>> I'm currently working in this area.  What pattern is causing
the cycle?
>>> Can I get a testcase?
>> 
>> I'll see if I can generate one and file a PR.
> 
> Ah, isLegalToFold saves us on trunk.  But we lose folding due to
prefetching,
> which is unfortunate.
> 
> I am seeing the error with 2.5 (yes, we are upgrading!).
> 
> I guess I'll have to backport some of the isLogalToFold logic.
Ok.  isLegalToFold will go away and be replaced with an even better mechanism
when my isel rewrite is done. :)

-Chris

Hello, David
> Ah, isLegalToFold saves us on trunk.  But we lose folding due to
prefetching,
> which is unfortunate.
>
> I am seeing the error with 2.5 (yes, we are upgrading!).
>
> I guess I'll have to backport some of the isLogalToFold logic.
There was x86-only code at pre-2.6 times which was later moved into
generic hook named "isLegalAndProfitableToFold". You might want to
backport just that part.

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

On Monday 22 February 2010 15:44:56 Anton Korobeynikov
wrote:
> Hello, David
>
> > Ah, isLegalToFold saves us on trunk.  But we lose folding due to
> > prefetching, which is unfortunate.
> >
> > I am seeing the error with 2.5 (yes, we are upgrading!).
> >
> > I guess I'll have to backport some of the isLogalToFold logic.
>
> There was x86-only code at pre-2.6 times which was later moved into
> generic hook named "isLegalAndProfitableToFold". You might want
to
> backport just that part.
I found the problem.  It's a bug in 2.5:

/// SelectScalarSSELoad - Match a scalar SSE load.  In particular, we want to
/// match a load whose top elements are either undef or zeros.  The load 
flavor
/// is derived from the type of N, which is either v4f32 or v2f64.
bool X86DAGToDAGISel::SelectScalarSSELoad(SDValue Op, SDValue Pred,
                                          SDValue N, SDValue &Base,
                                          SDValue &Scale, SDValue
&Index,
                                          SDValue &Disp, SDValue
&InChain,
                                          SDValue &OutChain) {
  if (N.getOpcode() == ISD::SCALAR_TO_VECTOR) {
    InChain = N.getOperand(0).getValue(1);
    if (ISD::isNON_EXTLoad(InChain.getNode()) &&
        InChain.getValue(0).hasOneUse() &&
        N.hasOneUse() &&
        // Cray: Bug 757517 (fixed in 2.7, possibly 2.6)
        IsLegalAndProfitableToFold(N.getOperand(0).getNode(), Pred.getNode(), 
Op.getNode())) {

The call used to be:

IsLegalAndProfitableToFold(N.getNode(), Pred.getNode(), Op.getNode()))

so it was checking the wrong thing for legality.

Thanks for the help, everyone!

                                          -Dave