[PATCH] D38288: [InstCombine] Restrict transforming shared selects using SimplifySelectsFeedingBinaryOp when we cannot simplify the binary op.

[Chad Rosier via Phabricator via llvm-commits]

mcrosier added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:740
     Value *V2 = SimplifyBinOp(Opcode, B, D, SQ.getWithInstruction(&I));
     if (V1 && V2)
       SI = Builder.CreateSelect(A, V2, V1);
----------------
mcberg2017 wrote:
> mcrosier wrote:
> > I wonder if something like this would work:
> > 
> >     bool SelectsHaveOneUse = LHS->hasOneUse() && RHS->hasOneUse();
> >     if (V1 && V2)
> >       SI = Builder.CreateSelect(A, V2, V1);
> >     else if (V2 && SelectsHaveOneUse)
> >       SI = Builder.CreateSelect(A, V2, Builder.CreateBinOp(Opcode, C, E));
> >     else if (V1 && SelectsHaveOneUse)
> >       SI = Builder.CreateSelect(A, Builder.CreateBinOp(Opcode, B, D), V1);
> An earlier review was opened here: https://reviews.llvm.org/D38263, from which the snippet above originates.  Just to let folks know that are two reviews open on this topic.
Somehow I did a drive by comment on D38263 without realizing the two fixes were the same.  I'd probably just go ahead and abandon this revision, Balaram.


https://reviews.llvm.org/D38288