[PATCH] D113442: [InstCombine] Enable fold select into operand for FAdd, FMul, and FSub.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D113442#3119977 <https://reviews.llvm.org/D113442#3119977>, @huihuiz wrote:

> I have some concern when I checked with alive2, for fadd https://alive2.llvm.org/ce/z/UjAMM_  alive2 complaints mis-matched outputs.

For fadd, we need to use -0.0 as the binop identity constant. I'm not getting the online instance of Alive2 to verify this without timing out, but I think this should work given enough time:
https://alive2.llvm.org/ce/z/TtGDNR



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:257
   case Instruction::Sub:   // Can only fold on the amount subtracted.
+  case Instruction::FSub:
   case Instruction::Shl:   // Can only fold on the shift amount.
----------------
huihuiz wrote:
> spatel wrote:
> > Why exclude fdiv?
> I checked with alive2, looks like adding fdiv, sdiv and udiv will trigger undefined behavior
> https://alive2.llvm.org/ce/z/KvFYev
>  
I think the integer ops are not safe because we can trigger immediate UB that might have been avoided in the original code, but fdiv should be fine - it doesn't have any different UB characteristics vs. fadd/fmul/fsub in the default FP environment.
https://alive2.llvm.org/ce/z/Yj_NA2
(This is timing out on the online version when allowing undef/poison, so please double-check on a local machine.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113442