[PATCH] Reorder shuffle and binary operation.

[James Molloy]

Hi Serge,

This patch has been breaking our internal builders since Sunday - it breaks
all random vector intrinsics tests.

Thanks for responding so quickly to the first reported bug but we've had one
outstanding since Monday morning. As it is now Wednesday, I think unless you
can address this soon we should start thinking about reverting this change
until you fix it.

Do you have any idea when you could get around to it?

Cheers,

James

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Hao Liu
> Sent: 14 May 2014 07:30
> 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.
> 
> 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
> 
> 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