[PATCH] Reorder shuffle and binary operation.

[Nadav Rotem]

I did not review the code carefully but I think it looks fine. We are not generating new shuffle masks so there should not be any lowering problems. 


On May 9, 2014, at 11:17 AM, Andrea Di Biagio <Andrea_DiBiagio at sn.scee.net> wrote:

> ================
> 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
> 
>