[PATCH] D50714: [InstCombine] Fold Select with binary op - FP opcodes

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:72-73
+    IsEq = true;
+  else if (Pred == FCmpInst::FCMP_ONE)
+    IsEq = false;
+  else
----------------
Did you try testing ONE/UNE in alive-fp?


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:91
 
+  // Check for signed zeros
+  if (isa<FPMathOperator>(BO) && !BO->hasNoSignedZeros())
----------------
This comment should explain, not just repeat the code:
+0.0 compares equal to -0.0, and so it does not behave as required for this transform. Bail out if we can not exclude that possibility.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:92
+  // Check for signed zeros
+  if (isa<FPMathOperator>(BO) && !BO->hasNoSignedZeros())
+    if (!CannotBeNegativeZero(Y, &TLI))
----------------
I don't think you need to have these checks? But I suppose checking for FPMathOperator is a good avoidance for calling CannotBeNegativeZero for the common (integer) case.


https://reviews.llvm.org/D50714