[PATCH] Fix PR10475

[Michael Liao]

Hi Nadav

On Thu, 2013-02-21 at 09:12 -0800, Nadav Rotem wrote:
> I think that we should make sure that the shifted value and the amount are either both vectors or both scalars.  I looked at it in the past and wanted to add assertions, but the Cell backend relied on this behavior. But now that the Cell backend is gone we can add the assertions and catch all of the places that violate this rule. 

That will break all shifts generated within TICG. All code there call
TLI.getShiftAmountTy() to create the shift amount operand. But all
shifts generated in TICG fall into vector-scalar shift form. There's
another issue by forcing that. It will also break lots of assertions
assuming ShiftAmountTy() to be MVT instead of EVT. With DAG preparation,
the vector shift amount may have to an EVT (illegal type mostly.)

Allowing shift-amount to be scalar or the same vector type if the 1st
operand is an integer vector may has the minor impact for the existing
code comparing ensuring them to be either both vectors or both scalars.
What's your thoughts?

Yours
- Michael

> 
> Thanks,
> Nadav 
> 
> On Feb 20, 2013, at 11:52 PM, Michael Liao <michael.liao at intel.com> wrote:
> 
> > Hi All,
> > 
> > The root cause of PR10475 is the X86 vector shift lowering logic assumes
> > the shift amount operand to be a vector. But all shift nodes generated
> > by generic DAG combination/optimization, e.g. (Z-X) == X --> Z == X<<1,
> > have scalar shift amount.
> > 
> > The documentation on ISD::SHL doesn't explicitly state that there's no
> > such restriction on scalar or vector. (You could guess that since it
> > mentioned it should be of type TLI.getShiftAmountTy(), which always
> > return scalar type on all targets so far.) This may cause
> > misunderstanding as 'shl' in LLVM IR does have the restriction as "Both
> > arguments to the ‘shl‘ instruction must be the same integer or vector of
> > integer type." I'd like to revise the document if we think it's
> > necessary.
> > 
> > BTW, another thought is that if shift-amount is allowed to be a scalar,
> > we may add a generic DAG combination to detect cases where a vector
> > shift-amount could be treated as a scalar. This could save all targets
> > with similar vector-scalar-shift ops.
> > 
> > Yours
> > - Michael
> > <0001-Fix-PR10475.patch>_______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>