[InstructionSimplify] handle denormal constant input for fcmp

[shchenz]

Summary

Handle denormal input for fcmp instructions according to denormal float handling mode.

[dcandler]

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

llvm/lib/Analysis/ConstantFolding.cpp

1349

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.

llvm/lib/Analysis/InstructionSimplify.cpp

3896–3897

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.

3909

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.

[shchenz]

address @dcandler comments

[shchenz]

Thanks for review @dcandler . Updated accordingly.

llvm/lib/Analysis/ConstantFolding.cpp

1349

Good point. Done.

llvm/lib/Analysis/InstructionSimplify.cpp

3896–3897

I chose not to use SimplifyQuery Q before is because the instruction in Q may be nullptr. Seems using the instruction inside Q also does not impact the motivated case, so I use it now.

3909

Sure. I change to handle denormal input in ConstantFoldCompareInstOperands now. However ConstantFoldCompareInstOperands is not just for float compare, it is also for integer compare. So it also makes to me that we don't add a parameter specific for float in the function.

[spatel]

llvm/lib/Analysis/ConstantFolding.cpp

1385

Prefer dyn_cast rather than isa+cast. So it could be written like this:

// Flush denormal inputs if needed.
if (auto *LCFP = dyn_cast<ConstantFP>(LHS)) {
  const fltSemantics &FltSema = LCFP->getType()->getFltSemantics();
  LHS = FlushFPConstant(LCFP, F->getDenormalMode(FltSema).Input);
}
if (auto *RCFP = dyn_cast<ConstantFP>(RHS)) {
  const fltSemantics &FltSema = RCFP->getType()->getFltSemantics();
  RHS = FlushFPConstant(RCFP, F->getDenormalMode(FltSema).Input);
}

// Calculate constant result.
Constant *C = ConstantFoldBinaryOpOperands(Opcode, LHS, RHS, DL);

// Flush denormal output if needed.
if (auto *CFP = dyn_cast<ConstantFP>(C)) {
  const fltSemantics &FltSema = CFP->getType()->getFltSemantics();
  C = FlushFPConstant(CFP, F->getDenormalMode(FltSema).Output);
}

return C;

llvm/test/Transforms/InstSimplify/constant-fold-fp-denormal.ll

763

Please add more tests to verify that we have the expected behavior for other denormal modes. These would be similar to what was added with D116952 (see tests in this file above here).

773

This is identical to the previous test?

[dcandler]

llvm/lib/Analysis/InstructionSimplify.cpp

3896–3897

In the previous review the view was that if there is no parent instruction/function available to get the denormal mode from, then it is okay to default to IEEE.

3909

ConstantFoldCompareInstOperands does do both float and integer compares, so my thinking was to only start worrying about denormals inside that function after we can establish whether it is dealing with integer or float instructions. That was why I suggested using an instruction pointer as an additional parameter: regardless of the type, a pointer to the instruction being folding should be available in most cases, and then this can be refined down to the denormal mode within the function only when needed.

This would make it easier to update calls to ConstantFoldCompareInstOperands elsewhere (e.g. ConstantFold in SimplifyCFG.cpp), since the existing instruction pointer can just be passed down, rather than having to determine a denormal mode for every usage, especially when a denormal mode may not be needed.

llvm/test/Transforms/InstSimplify/constant-fold-fp-denormal.ll

789

Since compares only take floating point inputs, another relevant test would be to ensure compares don't get folded when only an output mode is set (attributes #1 and #2) but do get folded on an input mode (#3 and #4).

[shchenz]

address comments

[shchenz]

Thanks. @spatel @dcandler

Could you please help to have another look?

llvm/lib/Analysis/InstructionSimplify.cpp

3909

Fair.

[spatel]

llvm/include/llvm/Analysis/ConstantFolding.h

72–80

Since we are updating this comment, please remove the function name at the start:
"Don’t duplicate function or class name at the beginning of the comment."
https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments

Also, change the wording slightly:

For fcmp, denormal inputs may be flushed based on the denormal handling mode.

llvm/test/Transforms/InstSimplify/constant-fold-fp-denormal.ll

752–753

Here and in other tests - I didn't recognize the hex value as a denorm at first sight. It would be easier to see with the leading zeros shown explicitly.

773

Is this still the same as the previous test? It would be better to make the denorm constant as operand 0 and maybe vary the fcmp predicate for better coverage.

[shchenz]

address @spatel comments

• add more test
• comments format fix

llvm/include/llvm/Analysis/ConstantFolding.h

72–80

Thanks for remind.

llvm/test/Transforms/InstSimplify/constant-fold-fp-denormal.ll

773

Sorry, the attribute here should be #4.

[spatel]

LGTM - but please wait a bit in case @dcandler has any other comments.

llvm/include/llvm/Analysis/ConstantFolding.h

82

denormal -> Denormal

[dcandler]

LGTM too, thanks.

[shchenz]

address comments and rebase

llvm/include/llvm/Analysis/ConstantFolding.h

 // DataLayout information. These functions cannot go in VMCore due to library
 // dependency issues.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ANALYSIS_CONSTANTFOLDING_H
 #define LLVM_ANALYSIS_CONSTANTFOLDING_H
 
+#include "llvm/ADT/APFloat.h"
 #include <stdint.h>
 
 namespace llvm {
 class APInt;
 template <typename T> class ArrayRef;
 class CallBase;
 class Constant;
+class ConstantFP;
 class DSOLocalEquivalent;
 class DataLayout;
 class Function;
 class GlobalValue;
 class GlobalVariable;
 class Instruction;
 class TargetLibraryInfo;
 class Type;

 /// null is returned.  Note that this function can fail when attempting to
 /// fold instructions like loads and stores, which have no constant expression
 /// form.
 ///
 Constant *ConstantFoldInstOperands(Instruction *I, ArrayRef<Constant *> Ops,
                                    const DataLayout &DL,
                                    const TargetLibraryInfo *TLI = nullptr);
 
 /// ConstantFoldCompareInstOperands - Attempt to constant fold a compare
 /// instruction (icmp/fcmp) with the specified operands.  If it fails, it
 /// returns a constant expression of the specified operands.
 ///
 Constant *
 ConstantFoldCompareInstOperands(unsigned Predicate, Constant *LHS,
                                 Constant *RHS, const DataLayout &DL,
                                 const TargetLibraryInfo *TLI = nullptr);
 
 /// Attempt to constant fold a unary operation with the specified
 /// operand. If it fails, it returns a constant expression of the specified
 /// operands.
 Constant *ConstantFoldUnaryOpOperand(unsigned Opcode, Constant *Op,
                                      const DataLayout &DL);
 
 /// Attempt to constant fold a binary operation with the specified
 /// operands.  If it fails, it returns a constant expression of the specified
 /// operands.
 Constant *ConstantFoldBinaryOpOperands(unsigned Opcode, Constant *LHS,
                                        Constant *RHS, const DataLayout &DL);
 
 /// Attempt to constant fold a floating point binary operation with the
 /// specified operands, applying the denormal handling mod to the operands.  If
 /// it fails, it returns a constant expression of the specified operands.
 Constant *ConstantFoldFPInstOperands(unsigned Opcode, Constant *LHS,
                                      Constant *RHS, const DataLayout &DL,
                                      const Instruction *I);
 
+/// Attempt to flush float point constant according to function's denormal mode.
+ConstantFP *FlushFPConstant(ConstantFP *Operand, DenormalMode DenormMode,
+                            bool IsOutput);
+
 /// Attempt to constant fold a select instruction with the specified
 /// operands. The constant result is returned if successful; if not, null is
 /// returned.
 Constant *ConstantFoldSelectInstruction(Constant *Cond, Constant *V1,
                                         Constant *V2);
 
 /// Attempt to constant fold a cast with the specified operand.  If it
 /// fails, it returns a constant expression of the specified operand.

llvm/include/llvm/Analysis/InstructionSimplify.h

 Value *simplifyXorInst(Value *LHS, Value *RHS, const SimplifyQuery &Q);
 
 /// Given operands for an ICmpInst, fold the result or return null.
 Value *simplifyICmpInst(unsigned Predicate, Value *LHS, Value *RHS,
                         const SimplifyQuery &Q);
 
 /// Given operands for an FCmpInst, fold the result or return null.
 Value *simplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
-                        FastMathFlags FMF, const SimplifyQuery &Q);
+                        FastMathFlags FMF, const SimplifyQuery &Q,
+                        DenormalMode DenormMode);
 
 /// Given operands for a SelectInst, fold the result or return null.
 Value *simplifySelectInst(Value *Cond, Value *TrueVal, Value *FalseVal,
                           const SimplifyQuery &Q);
 
 /// Given operands for a GetElementPtrInst, fold the result or return null.
 Value *simplifyGEPInst(Type *SrcTy, Value *Ptr, ArrayRef<Value *> Indices,
                        bool InBounds, const SimplifyQuery &Q);

llvm/lib/Analysis/ConstantFolding.cpp

 // Also, to supplement the basic IR ConstantExpr simplifications,
 // this file defines some additional folding routines that can make use of
 // DataLayout information. These functions cannot go in IR due to library
 // dependency issues.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/ConstantFolding.h"
-#include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/TargetFolder.h"
Constant *llvm::ConstantFoldBinaryOpOperands(unsigned Opcode, Constant *LHS,
   return ConstantExpr::get(Opcode, LHS, RHS);
 }
 
 // Check whether a constant is a floating point denormal that should be flushed
 // to zero according to the denormal handling mode set in the function
 // attributes. If so, return a zero with the correct sign, otherwise return the
 // original constant. Inputs and outputs to floating point instructions can have
 // their mode set separately, so the direction is also needed.
-Constant *FlushFPConstant(Constant *Operand, const llvm::Function *F,
+ConstantFP *llvm::FlushFPConstant(ConstantFP *Operand, DenormalMode DenormMode,
                                   bool IsOutput) {
-  if (F == nullptr)
-    return Operand;
-  if (auto *CFP = dyn_cast<ConstantFP>(Operand)) {
-    const APFloat &APF = CFP->getValueAPF();
-    Type *Ty = CFP->getType();
-    DenormalMode DenormMode = F->getDenormalMode(Ty->getFltSemantics());
-    DenormalMode::DenormalModeKind Mode =
-        IsOutput ? DenormMode.Output : DenormMode.Input;
-    switch (Mode) {
-    default:
-      llvm_unreachable("unknown denormal mode");
-      return Operand;
-    case DenormalMode::IEEE:
-      return Operand;
-    case DenormalMode::PreserveSign:
-      if (APF.isDenormal()) {
-        return ConstantFP::get(
-            Ty->getContext(),
-            APFloat::getZero(Ty->getFltSemantics(), APF.isNegative()));
-      }
-      return Operand;
-    case DenormalMode::PositiveZero:
-      if (APF.isDenormal()) {
-        return ConstantFP::get(Ty->getContext(),
-                               APFloat::getZero(Ty->getFltSemantics(), false));
-      }
-      return Operand;
-    }
-  }
+  const APFloat &APF = Operand->getValueAPF();
+  Type *Ty = Operand->getType();
   return Operand;
 }
 
 Constant *llvm::ConstantFoldFPInstOperands(unsigned Opcode, Constant *LHS,
                                            Constant *RHS, const DataLayout &DL,
                                            const Instruction *I) {
   if (auto *BB = I->getParent()) {
     if (auto *F = BB->getParent()) {
       if (Instruction::isBinaryOp(Opcode)) {
-        Constant *Op0 = FlushFPConstant(LHS, F, false);
-        Constant *Op1 = FlushFPConstant(RHS, F, false);
+        Constant *Op0 =
+            isa<ConstantFP>(LHS)
+                ? FlushFPConstant(
+                      cast<ConstantFP>(LHS),
+                      F->getDenormalMode(LHS->getType()->getFltSemantics()),
+                      false)
+                : LHS;
+        Constant *Op1 =
+            isa<ConstantFP>(RHS)
+                ? FlushFPConstant(
+                      cast<ConstantFP>(RHS),
+                      F->getDenormalMode(RHS->getType()->getFltSemantics()),
+                      false)
+                : RHS;
         Constant *C = ConstantFoldBinaryOpOperands(Opcode, Op0, Op1, DL);
-        return FlushFPConstant(C, F, true);
+        return isa<ConstantFP>(C)
+                   ? FlushFPConstant(
+                         cast<ConstantFP>(C),
+                         F->getDenormalMode(C->getType()->getFltSemantics()),
+                         true)
+                   : C;
       }
     }
   }
   // If instruction lacks a parent/function and the denormal mode cannot be
   // determined, use the default (IEEE).
   return ConstantFoldBinaryOpOperands(Opcode, LHS, RHS, DL);
 }
 

llvm/lib/Analysis/InstructionSimplify.cpp

 Value *llvm::simplifyICmpInst(unsigned Predicate, Value *LHS, Value *RHS,
                               const SimplifyQuery &Q) {
   return ::simplifyICmpInst(Predicate, LHS, RHS, Q, RecursionLimit);
 }
 
 /// Given operands for an FCmpInst, see if we can fold the result.
 /// If not, this returns null.
 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;
   assert(CmpInst::isFPPredicate(Pred) && "Not an FP compare!");
 
   if (Constant *CLHS = dyn_cast<Constant>(LHS)) {
-    if (Constant *CRHS = dyn_cast<Constant>(RHS))
+    if (Constant *CRHS = dyn_cast<Constant>(RHS)) {
+      if (isa<ConstantFP>(CLHS))
+        CLHS = FlushFPConstant(cast<ConstantFP>(CLHS), DenormMode,
+                               /* IsOutput */ false);
+      if (isa<ConstantFP>(CRHS))
+        CRHS = FlushFPConstant(cast<ConstantFP>(CRHS), DenormMode,
+                               /* IsOutput */ false);
       return ConstantFoldCompareInstOperands(Pred, CLHS, CRHS, Q.DL, Q.TLI);
+    }
 
     // If we have a constant, make sure it is on the RHS.
     std::swap(LHS, RHS);
     Pred = CmpInst::getSwappedPredicate(Pred);
   }
 
   // Fold trivial predicates.
   Type *RetTy = getCompareTy(LHS);
static Value *simplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
   if (isa<PHINode>(LHS) || isa<PHINode>(RHS))
     if (Value *V = threadCmpOverPHI(Pred, LHS, RHS, Q, MaxRecurse))
       return V;
 
   return nullptr;
 }
 
 Value *llvm::simplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
-                              FastMathFlags FMF, const SimplifyQuery &Q) {
-  return ::simplifyFCmpInst(Predicate, LHS, RHS, FMF, Q, RecursionLimit);
+                              FastMathFlags FMF, const SimplifyQuery &Q,
+                              DenormalMode DenormMode) {
+  return ::simplifyFCmpInst(Predicate, LHS, RHS, FMF, Q, RecursionLimit,
+                            DenormMode);
 }
 
 static Value *simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
                                      const SimplifyQuery &Q,
                                      bool AllowRefinement,
                                      unsigned MaxRecurse) {
   assert(!Op->getType()->isVectorTy() && "This is not safe for vectors");
 
Value *llvm::simplifyBinOp(unsigned Opcode, Value *LHS, Value *RHS,
   return ::simplifyBinOp(Opcode, LHS, RHS, FMF, Q, RecursionLimit);
 }
 
 /// Given operands for a CmpInst, see if we can fold the result.
 static Value *simplifyCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
                               const SimplifyQuery &Q, unsigned MaxRecurse) {
   if (CmpInst::isIntPredicate((CmpInst::Predicate)Predicate))
     return simplifyICmpInst(Predicate, LHS, RHS, Q, MaxRecurse);
-  return simplifyFCmpInst(Predicate, LHS, RHS, FastMathFlags(), Q, MaxRecurse);
+  // For now, most denormal float constants folding should be done in
+  // simplifyFCmpInst(), so simply pass IEEE mode for both input and output
+  // here.
+  // FIXME: use the real denormal mode from the function's attributes.
+  return simplifyFCmpInst(Predicate, LHS, RHS, FastMathFlags(), Q, MaxRecurse,
+                          {DenormalMode::IEEE, DenormalMode::IEEE});
 }
 
 Value *llvm::simplifyCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
                              const SimplifyQuery &Q) {
   return ::simplifyCmpInst(Predicate, LHS, RHS, Q, RecursionLimit);
 }
 
 static bool isIdempotent(Intrinsic::ID ID) {
case Instruction::Or:
     break;
   case Instruction::Xor:
     Result = simplifyXorInst(NewOps[0], NewOps[1], Q);
     break;
   case Instruction::ICmp:
     Result = simplifyICmpInst(cast<ICmpInst>(I)->getPredicate(), NewOps[0],
                               NewOps[1], Q);
     break;
-  case Instruction::FCmp:
+  case Instruction::FCmp: {
+    DenormalMode Mode = {DenormalMode::IEEE, DenormalMode::IEEE};
+    // We can further fold denormal constant fp with specific denormal mode.
+    if (isa<ConstantFP>(NewOps[0]) && I->getParent())
+      Mode = I->getFunction()->getDenormalMode(
+          NewOps[0]->getType()->getFltSemantics());
     Result = simplifyFCmpInst(cast<FCmpInst>(I)->getPredicate(), NewOps[0],
-                              NewOps[1], I->getFastMathFlags(), Q);
+                              NewOps[1], I->getFastMathFlags(), Q, Mode);
     break;
+  }
   case Instruction::Select:
     Result = simplifySelectInst(NewOps[0], NewOps[1], NewOps[2], Q);
     break;
   case Instruction::GetElementPtr: {
     auto *GEPI = cast<GetElementPtrInst>(I);
     Result =
         simplifyGEPInst(GEPI->getSourceElementType(), NewOps[0],
                         makeArrayRef(NewOps).slice(1), GEPI->isInBounds(), Q);

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

Instruction *InstCombinerImpl::visitFCmpInst(FCmpInst &I) {
   /// before binary operators.
   if (getComplexity(I.getOperand(0)) < getComplexity(I.getOperand(1))) {
     I.swapOperands();
     Changed = true;
   }
 
   const CmpInst::Predicate Pred = I.getPredicate();
   Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);
+  DenormalMode Mode = {DenormalMode::IEEE, DenormalMode::IEEE};
+  if (isa<ConstantFP>(Op0) && I.getParent())
+    Mode = I.getFunction()->getDenormalMode(Op0->getType()->getFltSemantics());
   if (Value *V = simplifyFCmpInst(Pred, Op0, Op1, I.getFastMathFlags(),
-                                  SQ.getWithInstruction(&I)))
+                                  SQ.getWithInstruction(&I), Mode))
     return replaceInstUsesWith(I, V);
 
   // Simplify 'fcmp pred X, X'
   Type *OpType = Op0->getType();
   assert(OpType == Op1->getType() && "fcmp with different-typed operands?");
   if (Op0 == Op1) {
     switch (Pred) {
       default: break;

llvm/test/Transforms/InstSimplify/constant-fold-fp-denormal.ll

 ; CHECK-LABEL: @fcmp_double(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    ret i1 true
 ;
 entry:
   %cmp = fcmp une double 0x0, 0x8000000000000
   ret i1 %cmp
 }
 
 define i1 @fcmp_float() #0 {
 ; CHECK-LABEL: @fcmp_float(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    ret i1 true
 ;
 entry:
   %cmp = fcmp une double 0x0, 0x8000000000000
   ret i1 %cmp
 }
 
 define i1 @fcmp_double_positive_zero() #6 {
 ; CHECK-LABEL: @fcmp_double_positive_zero(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    ret i1 false
 ;
 entry:
   %cmp = fcmp une double 0x0, 0x8000000000000
   ret i1 %cmp
 }
 
 define i1 @fcmp_float_positive_zero() #6 {
 ; CHECK-LABEL: @fcmp_float_positive_zero(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    ret i1 false
 ;
 entry:
   %cmp = fcmp une double 0x0, 0x8000000000000
   ret i1 %cmp
 }
 
 attributes #0 = { nounwind "denormal-fp-math"="ieee,ieee" }
 attributes #1 = { nounwind "denormal-fp-math"="positive-zero,ieee" }
 attributes #2 = { nounwind "denormal-fp-math"="preserve-sign,ieee" }
 attributes #3 = { nounwind "denormal-fp-math"="ieee,positive-zero" }
 attributes #4 = { nounwind "denormal-fp-math"="ieee,preserve-sign" }
 attributes #5 = { nounwind "denormal-fp-math"="ieee,ieee" "denormal-fp-math-f32"="positive-zero,ieee" }
 attributes #6 = { nounwind "denormal-fp-math"="positive-zero,positive-zero" }