[PATCH] Reorder shuffle and binary operation.

[Hao Liu]

Hi Serge,

It seems that there is another bug http://llvm.org/bugs/show_bug.cgi?id=19730.
When two shuffles have different types and the same mask, there may be an assertion failure.

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


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

http://reviews.llvm.org/D3525