[PATCH] Reorder shuffle and binary operation.

[Hao Liu]

Hi Serge,

It seems that your patch introduces a bug when two shuffles have different
operand types.
See more details in http://llvm.org/bugs/show_bug.cgi?id=19717.

Thanks,
-Hao

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Serge Pavlov
> Sent: Sunday, May 11, 2014 4:42 PM
> To: sepavloff at gmail.com; eli.friedman at gmail.com; benny.kra at gmail.com;
> nrotem at apple.com
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Reorder shuffle and binary operation.
> 
> Thank you for advice and detailed explanation!
> I updated the patch according your recommendation.
> 
> --Serge
> 
> 
> 2014-05-10 1:17 GMT+07:00 Andrea Di Biagio
> <Andrea_DiBiagio at sn.scee.net>:
> 
> > ================
> > Comment at:
> > lib/Transforms/InstCombine/InstructionCombining.cpp:1123-1125
> > @@ +1122,5 @@
> > +        isa<UndefValue>(RShuf->getOperand(1))) {
> > +      SmallVector<int, 16> LHSMask = LShuf->getShuffleMask();
> > +      SmallVector<int, 16> RHSMask = RShuf->getShuffleMask();
> > +      if (std::equal(LHSMask.begin(), LHSMask.end(), RHSMask.begin()))
{
> > +        BinaryOperator *NewBO = CreateBinOpAsGiven(Inst,
> > LShuf->getOperand(0),
> > ----------------
> > I might be wrong but,
> > wouldn't be easier in this case to simply compare the addresses of the
> > two shuffle masks?
> >
> > As far as I understand, the shuffle mask (third operand of a
> > ShuffleVectorInst) is always a Constant; therefore, you should be able
> > to check if two masks are equal by simply comparing their addresses. I
> > expect that something like this should work:
> >
> > cast<ShuffleVectorInst>(LHS)->getMask() ==
> > cast<ShuffleVectorInst>(RHS)->getMask().
> >
> > quoting what is written in Constant.h, "two structurally equivalent
> > constants will always have the same address". Also, "The shuffle mask
> > operand is required to be a constant vector with either constant
> > integer or undef values".
> >
> > I hope this helps.
> >
> > http://reviews.llvm.org/D3525
> >
> >
> >
> 
> http://reviews.llvm.org/D3525
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

http://reviews.llvm.org/D3525