[PATCH] Reorder shuffle and binary operation.

Serge Pavlov sepavloff at gmail.com
Sun May 11 01:42:29 PDT 2014


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






More information about the llvm-commits mailing list