[PATCH] D84664: [InstCombine] Fold shift-select if all the operands are constant

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Okay, after looking at this again I think you're on the right track here.



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:879
+      }
+    }
   }
----------------
I have two questions here:

First, can this change be tested independently of the change to shift operators? This looks pretty orthogonal and should be testable through existing uses of SimplifySelectsFeedingBinaryOp.

Second, I don't think that checking for the constant case specifically is the right thing to do. It should be mentioned that just dropping the hasOneUse() checks above would be correct and end up replacing a binop with a select -- and I guess a canonicality decision has been made here to not do that.

If we want to still allow the replacement if the select has constant operands (which seems to be the intention here), then we should drop the hasOneUse() checks above, and instead check whether //either// the select has one use, or True and False folded to constants. This may occur even if the original operands were not constants.


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

https://reviews.llvm.org/D84664