[PATCH] D128647: [InstructionSimplify] handle denormal constant input for fcmp

[David Candler via Phabricator via llvm-commits]

dcandler added a comment.

Thanks, I'd started looking at how to include fcmp but you were quicker.



================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1349
 // their mode set separately, so the direction is also needed.
-Constant *FlushFPConstant(Constant *Operand, const llvm::Function *F,
-                          bool IsOutput) {
-  if (F == nullptr)
+ConstantFP *llvm::FlushFPConstant(ConstantFP *Operand, DenormalMode DenormMode,
+                                  bool IsOutput) {
----------------
If the denormal mode is used as an argument instead of a function pointer to obtain it, then DenormMode.Input or DenormMode.Output can be used directly instead of the mode struct plus a selector.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3896-3897
 static Value *simplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
                                FastMathFlags FMF, const SimplifyQuery &Q,
-                               unsigned MaxRecurse) {
+                               unsigned MaxRecurse, DenormalMode DenormMode) {
   CmpInst::Predicate Pred = (CmpInst::Predicate)Predicate;
----------------
The SimplifyQuery Q has a pointer to the instruction, so the denormal mode can be obtained via its parent rather than needing the denormal mode as an additional argument.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3909
+                               /* IsOutput */ false);
       return ConstantFoldCompareInstOperands(Pred, CLHS, CRHS, Q.DL, Q.TLI);
+    }
----------------
I feel it would be better to handle the denormals inside ConstantFoldCompareInstOperands (it would only need the instruction pointer from Q) as that would correct the result for all compare folding, whereas this patch currently only corrects the call from InstSimplify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128647