[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–1351

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.

3903–3904

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–1351

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.

3903–3904

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

1390

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

822–823

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).

832–833

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.

3903–3904

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

869–871

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

3903–3904

Fair.

[spatel]

llvm/include/llvm/Analysis/ConstantFolding.h

70–74

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

769

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.

832–833

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

70–74

Thanks for remind.

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

832–833

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

76

denormal -> Denormal

[dcandler]

LGTM too, thanks.

[shchenz]

address comments and rebase

llvm/include/llvm/Analysis/ConstantFolding.h

 /// 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);
+/// For fcmp, denormal constant input will be folded further according to the
+/// denormal handling mode.
+Constant *ConstantFoldCompareInstOperands(
+    unsigned Predicate, Constant *LHS, Constant *RHS, const DataLayout &DL,
+    const TargetLibraryInfo *TLI = nullptr, const Instruction *I = 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 denormal mode set in the
+/// instruction's parent 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 Instruction *I,
+                          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/lib/Analysis/ConstantFolding.cpp

for (const Use &OpU : I->operands()) {
     auto *Op = cast<Constant>(&OpU);
     // Fold the Instruction's operands.
     Op = ConstantFoldConstantImpl(Op, DL, TLI, FoldedOps);
     Ops.push_back(Op);
   }
 
   if (const auto *CI = dyn_cast<CmpInst>(I))
     return ConstantFoldCompareInstOperands(CI->getPredicate(), Ops[0], Ops[1],
-                                           DL, TLI);
+                                           DL, TLI, CI);
 
   if (const auto *LI = dyn_cast<LoadInst>(I)) {
     if (LI->isVolatile())
       return nullptr;
     return ConstantFoldLoadFromConstPtr(Ops[0], LI->getType(), DL);
   }
 
   if (auto *IVI = dyn_cast<InsertValueInst>(I))

 
 Constant *llvm::ConstantFoldInstOperands(Instruction *I,
                                          ArrayRef<Constant *> Ops,
                                          const DataLayout &DL,
                                          const TargetLibraryInfo *TLI) {
   return ConstantFoldInstOperandsImpl(I, I->getOpcode(), Ops, DL, TLI);
 }
 
-Constant *llvm::ConstantFoldCompareInstOperands(unsigned IntPredicate,
-                                                Constant *Ops0, Constant *Ops1,
-                                                const DataLayout &DL,
-                                                const TargetLibraryInfo *TLI) {
+Constant *llvm::ConstantFoldCompareInstOperands(
+    unsigned IntPredicate, Constant *Ops0, Constant *Ops1, const DataLayout &DL,
+    const TargetLibraryInfo *TLI, const Instruction *I) {
   CmpInst::Predicate Predicate = (CmpInst::Predicate)IntPredicate;
   // fold: icmp (inttoptr x), null         -> icmp x, 0
   // fold: icmp null, (inttoptr x)         -> icmp 0, x
   // fold: icmp (ptrtoint x), 0            -> icmp x, null
   // fold: icmp 0, (ptrtoint x)            -> icmp null, x
   // fold: icmp (inttoptr x), (inttoptr y) -> icmp trunc/zext x, trunc/zext y
   // fold: icmp (ptrtoint x), (ptrtoint y) -> icmp x, y
   //
if (auto *CE0 = dyn_cast<ConstantExpr>(Ops0)) {
     }
   } else if (isa<ConstantExpr>(Ops1)) {
     // If RHS is a constant expression, but the left side isn't, swap the
     // operands and try again.
     Predicate = ICmpInst::getSwappedPredicate(Predicate);
     return ConstantFoldCompareInstOperands(Predicate, Ops1, Ops0, DL, TLI);
   }
 
+  // Flush any denormal constant float input according to denormal handling
+  // mode.
+  Ops0 = FlushFPConstant(Ops0, I, /* IsOutput */ false);
+  Ops1 = FlushFPConstant(Ops1, I, /* IsOutput */ false);
+
   return ConstantExpr::getCompare(Predicate, Ops0, Ops1);
 }
 
 Constant *llvm::ConstantFoldUnaryOpOperand(unsigned Opcode, Constant *Op,
                                            const DataLayout &DL) {
   assert(Instruction::isUnaryOp(Opcode));
 
   return ConstantExpr::get(Opcode, Op);
 }
 
 Constant *llvm::ConstantFoldBinaryOpOperands(unsigned Opcode, Constant *LHS,
                                              Constant *RHS,
                                              const DataLayout &DL) {
   assert(Instruction::isBinaryOp(Opcode));
   if (isa<ConstantExpr>(LHS) || isa<ConstantExpr>(RHS))
     if (Constant *C = SymbolicallyEvaluateBinop(Opcode, LHS, RHS, DL))
       return C;
 
   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,
+Constant *llvm::FlushFPConstant(Constant *Operand, const Instruction *I,
                                 bool IsOutput) {
-  if (F == nullptr)
+  if (!I || !I->getParent() || !I->getFunction())
+    return Operand;
+
+  ConstantFP *CFP = dyn_cast<ConstantFP>(Operand);
+  if (!CFP)
     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;
-    }
-  }
+
+  DenormalMode DenormMode =
+      I->getFunction()->getDenormalMode(Ty->getFltSemantics());
   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 *C = ConstantFoldBinaryOpOperands(Opcode, Op0, Op1, DL);
-        return FlushFPConstant(C, F, true);
-      }
-    }
+    // Flush denormal inputs if needed.
+    Constant *Op0 = FlushFPConstant(LHS, I, /* IsOutput */ false);
+    Constant *Op1 = FlushFPConstant(RHS, I, /* IsOutput */ false);
+
+    // Calculate constant result.
+
+    // Flush denormal output if needed.
+    return FlushFPConstant(C, I, /* IsOutput */ true);
   }
   // If instruction lacks a parent/function and the denormal mode cannot be
   // determined, use the default (IEEE).
   return ConstantFoldBinaryOpOperands(Opcode, LHS, RHS, DL);
 }
 
 Constant *llvm::ConstantFoldCastOperand(unsigned Opcode, Constant *C,
                                         Type *DestTy, const DataLayout &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) {
   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))
-      return ConstantFoldCompareInstOperands(Pred, CLHS, CRHS, Q.DL, Q.TLI);
+      return ConstantFoldCompareInstOperands(Pred, CLHS, CRHS, Q.DL, Q.TLI,
+                                             Q.CxtI);
 
     // 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 *simplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
   // will be done in InstCombine).
   // TODO: This may be unsound, because it only catches some forms of
   // refinement.
   if (!AllowRefinement && canCreatePoison(cast<Operator>(I)))
     return nullptr;
 
   if (CmpInst *C = dyn_cast<CmpInst>(I))
     return ConstantFoldCompareInstOperands(C->getPredicate(), ConstOps[0],
-                                           ConstOps[1], Q.DL, Q.TLI);
+                                           ConstOps[1], Q.DL, Q.TLI, C);
 
   if (LoadInst *LI = dyn_cast<LoadInst>(I))
     if (!LI->isVolatile())
       return ConstantFoldLoadFromConstPtr(ConstOps[0], LI->getType(), Q.DL);
 
   return ConstantFoldInstOperands(I, ConstOps, Q.DL, Q.TLI);
 }
 

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

 entry:
   %cmp = fcmp une float 0x0, 0x3800000000000000
   ret i1 %cmp
 }
 
 define i1 @fcmp_double_pz_in_pz_out() #6 {
 ; CHECK-LABEL: @fcmp_double_pz_in_pz_out(
 ; 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_pz_in_pz_out() #6 {
 ; CHECK-LABEL: @fcmp_float_pz_in_pz_out(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    ret i1 false
 ;
 entry:
   %cmp = fcmp une float 0x0, 0x3800000000000000
   ret i1 %cmp
 }
 
 define i1 @fcmp_double_ps_in_ps_out() #7 {
 ; CHECK-LABEL: @fcmp_double_ps_in_ps_out(
 ; 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_ps_in_ps_out() #7 {
 ; CHECK-LABEL: @fcmp_float_ps_in_ps_out(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    ret i1 false
 ;
 entry:
   %cmp = fcmp une float 0x0, 0x3800000000000000
   ret i1 %cmp
 }
 
 define i1 @fcmp_double_pz_out_ieee_in() #1 {
 ; CHECK-LABEL: @fcmp_double_pz_out_ieee_in(

 ; CHECK-LABEL: @fcmp_double_ps_out_ieee_in(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    ret i1 true
 ;
 entry:
   %cmp = fcmp une double 0x0, 0x8000000000000
   ret i1 %cmp
 }
 
 define i1 @fcmp_double_ieee_out_pz_in() #3 {
 ; CHECK-LABEL: @fcmp_double_ieee_out_pz_in(
 ; 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_double_ieee_out_ps_in() #3 {
 ; CHECK-LABEL: @fcmp_double_ieee_out_ps_in(
 ; 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_double_f32_pz_in() #8 {
 ; CHECK-LABEL: @fcmp_double_f32_pz_in(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    ret i1 true
 ;
 entry:
   %cmp = fcmp une double 0x0, 0x8000000000000
   ret i1 %cmp
 }
 
 define i1 @fcmp_double_two_denormal_ins() #6 {
 ; CHECK-LABEL: @fcmp_double_two_denormal_ins(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    ret i1 false
 ;
 entry:
   %cmp = fcmp une double 0x8100000000000, 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" }
 attributes #7 = { nounwind "denormal-fp-math"="preserve-sign,preserve-sign" }
 attributes #8 = { nounwind "denormal-fp-math"="ieee,ieee" "denormal-fp-math-f32"="positive-zero,positive-zero" }