[PATCH] Reorder shuffle and binary operation.

[Hao Liu]

Hi Serge,

Sorry to bother you again. 
It seems there is still a bug http://llvm.org/bugs/show_bug.cgi?id=19737
when creating binary operator.
I've looked at your code. Except this minor bug, your patch may not have any
other problems.

Thanks,
-Hao

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Hao Liu
> Sent: Monday, May 12, 2014 11:20 AM
> To: reviews+D3525+public+b0fd3ed101ab4b75 at reviews.llvm.org;
> 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.
> 
> 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
> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits