[PATCH] D153963: [InstCombine] Fold binop of select and cast of select condition

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D153963#4486495 <https://reviews.llvm.org/D153963#4486495>, @nikic wrote:

> @goldstein.w.n The transform needs to be called from somewhere. Currently it is called for a number of binops. The same transform is indeed possible for non-binop instructions, but I'm not sure it makes sense to plaster it everywhere.

I see, thats a fair point.

> The main sensible way of doing that I can see is to extend FoldOpIntoSelect() to handle this. We could extend constantFoldOperationIntoSelectOperand() to determine the constant for zext/sext of the condition, similar to the special case for icmps it currently has. However, this would also require relaxing various checks where we currently only call FoldOpIntoSelect() if we have a constant operand.
>
> I guess if there is no compile-time impact to calling FoldOpIntoSelect() for non-constants, then doing that would be a pretty clean and general way to go about it.

Okay, guess we can leave that as a todo for the future. I may look into when I have the time.

@antoniofrighetto for now I think its okay to keep this only for binops, but please add a todo that
if `FoldOpIntoSelect` is updated, we should remove the binop specific function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153963/new/

https://reviews.llvm.org/D153963