ValueTracking: fadd +0 cannot return -0

[kpn]

llvm/lib/Analysis/ValueTracking.cpp

4369

We're in the default FP environment here since we're looking at FAdd and FSub instructions. The check for FAdd isn't needed?

[arsenm]

Also handle sub

llvm/lib/Analysis/ValueTracking.cpp

4369

I was just copying from CannotBeNegativeZero but I guess we can improve on it at the same time

[foad]

llvm/lib/Analysis/ValueTracking.cpp

4369

As I understand it, with default rounding the only addition that can return -0 is -0 + -0. So if either operand is known not to be -0, the result cannot be -0. And the only subtraction that can return -0 is -0 - +0.

[arsenm]

llvm/lib/Analysis/ValueTracking.cpp

4369

Checking what instcombine thinks,

define float @fsub_p0_p0(float %arg0) {
  %sub = fsub float 0.0, 0.0
  ret float %sub
}

define float @fsub_n0_n0(float %arg0) {
  %sub = fsub float -0.0, -0.0
  ret float %sub
}

define float @fsub_p0_n0(float %arg0) {
  %sub = fsub float 0.0, -0.0
  ret float %sub
}

define float @fsub_n0_p0(float %arg0) {
  %sub = fsub float -0.0, 0.0
  ret float %sub
}

define float @fsub_p0_p0(float %arg0) {
  ret float 0.000000e+00
}

define float @fsub_n0_n0(float %arg0) {
  ret float 0.000000e+00
}

define float @fsub_p0_n0(float %arg0) {
  ret float 0.000000e+00
}

define float @fsub_n0_p0(float %arg0) {
  ret float -0.000000e+00
}

The missing optimization we could do here is check if DAZ is enabled and also cover denormal inputs

[arsenm]

ping

[foad]

llvm/lib/Analysis/ValueTracking.cpp

4369

First, this is wrong for FSub, since -0 - +0 = -0

Second, you can generalize KnownRHS.KnownFPClasses == fcPosZero to KnownRHS.isKnownNot(fcNegZero) (or fcPosZero for FSub).

Third you could do a similar check on the LHS. For both FAdd and FSub, if the LHS is not -0 then the result can't be -0.

[arsenm]

Fix fsub and DAZ handling

[foad]

llvm/include/llvm/Analysis/ValueTracking.h

281 ↗

(On Diff #515550)

Matter of taste I guess, but I don't think a helper function this simple actually helps.

llvm/lib/Analysis/ValueTracking.cpp

3879–3880

Could use inputDenormalIsIEEEOrPosZero here.

3884

This doesn't work. If denormal mode is PosZero then a denormal can be flushed to +0, so this function should return false.

[arsenm]

Fix pos zero bug by just fully handling the denormal modes

[arsenm]

ping

llvm/include/llvm/Analysis/ValueTracking.h

281 ↗

(On Diff #515550)

Mostly for consistency with the others

[foad]

LGTM

llvm/lib/Analysis/ValueTracking.cpp

3888

Nit: might as well push this into the PositiveZero/default case.

[arsenm]

llvm/lib/Analysis/ValueTracking.cpp

3888

That wouldn't short circuit the attribute parsing

[arsenm]

c9f88e5bf027edcebd3ecbc2cebcda57887e04cf

llvm/lib/Analysis/ValueTracking.cpp

static bool inputDenormalIsIEEE(const Function &F, const Type *Ty) {
   Ty = Ty->getScalarType();
   return F.getDenormalMode(Ty->getFltSemantics()).Input == DenormalMode::IEEE;
 }
 
 bool KnownFPClass::isKnownNeverLogicalZero(const Function &F, Type *Ty) const {
   return isKnownNeverZero() &&
          (isKnownNeverSubnormal() || inputDenormalIsIEEE(F, Ty));
 }
 
 /// Returns a pair of values, which if passed to llvm.is.fpclass, returns the
 /// same result as an fcmp with the given operands.
 std::pair<Value *, FPClassTest> llvm::fcmpToClassTest(FCmpInst::Predicate Pred,
                                                       const Function &F,
                                                       Value *LHS, Value *RHS,
                                                       bool LookThroughSrc) {
   const APFloat *ConstRHS;
   if (!match(RHS, m_APFloat(ConstRHS)))
     return {nullptr, fcNone};
 
   if (ConstRHS->isZero()) {
     // Compares with fcNone are only exactly equal to fcZero if input denormals are
     // not flushed.
     if (FCmpInst::isEquality(Pred) && !inputDenormalIsIEEE(F, LHS->getType()))
       return {nullptr, fcNone};
 
     switch (Pred) {
case Instruction::Call: {
 
     break;
   }
   case Instruction::FAdd:
   case Instruction::FSub: {
     KnownFPClass KnownLHS, KnownRHS;
     computeKnownFPClass(Op->getOperand(1), DemandedElts, fcNan | fcInf,
                         KnownRHS, Depth + 1, Q, TLI);
+
+    // (fadd|fsub x, 0.0) is guaranteed to return +0.0, not -0.0.
+    if (KnownRHS.KnownFPClasses == fcPosZero)
+      Known.knownNot(fcNegZero);
+
     if (KnownRHS.isKnownNeverNaN()) {
       // RHS is canonically cheaper to compute. Skip inspecting the LHS if
       // there's no point.
       computeKnownFPClass(Op->getOperand(0), DemandedElts, fcNan | fcInf,
                           KnownLHS, Depth + 1, Q, TLI);
       // Adding positive and negative infinity produces NaN.
       // TODO: Check sign of infinities.
       if (KnownLHS.isKnownNeverNaN() &&

llvm/test/Transforms/Attributor/nofpclass.ll

 ; CHECK-LABEL: define nofpclass(nan inf nzero) float @sitofp
 ; CHECK-SAME: (i32 [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[CVT:%.*]] = sitofp i32 [[ARG]] to float
 ; CHECK-NEXT:    ret float [[CVT]]
 ;
   %cvt = sitofp i32 %arg to float
   ret float %cvt
 }
+
+define float @fadd_p0(float %arg0) {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define nofpclass(nzero) float @fadd_p0
+; CHECK-SAME: (float [[ARG0:%.*]]) #[[ATTR2]] {
+; CHECK-NEXT:    [[ADD:%.*]] = fadd float [[ARG0]], 0.000000e+00
+; CHECK-NEXT:    ret float [[ADD]]
+;
+  %add = fadd float %arg0, 0.0
+  ret float %add
+}
+
+define float @fadd_n0(float %arg0) {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define float @fadd_n0
+; CHECK-SAME: (float [[ARG0:%.*]]) #[[ATTR2]] {
+; CHECK-NEXT:    [[ADD:%.*]] = fadd float [[ARG0]], -0.000000e+00
+; CHECK-NEXT:    ret float [[ADD]]
+;
+  %add = fadd float %arg0, -0.0
+  ret float %add
+}
+
+define float @fsub_p0(float %arg0) {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define nofpclass(nzero) float @fsub_p0
+; CHECK-SAME: (float [[ARG0:%.*]]) #[[ATTR2]] {
+; CHECK-NEXT:    [[SUB:%.*]] = fsub float [[ARG0]], 0.000000e+00
+; CHECK-NEXT:    ret float [[SUB]]
+;
+  %sub = fsub float %arg0, 0.0
+  ret float %sub
+}
+
+define float @fsub_n0(float %arg0) {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define float @fsub_n0
+; CHECK-SAME: (float [[ARG0:%.*]]) #[[ATTR2]] {
+; CHECK-NEXT:    [[SUB:%.*]] = fsub float [[ARG0]], -0.000000e+00
+; CHECK-NEXT:    ret float [[SUB]]
+;
+  %sub = fsub float %arg0, -0.0
+  ret float %sub
+}
+
+define float @fsub_p0_commute(float %arg0) {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define float @fsub_p0_commute
+; CHECK-SAME: (float [[ARG0:%.*]]) #[[ATTR2]] {
+; CHECK-NEXT:    [[SUB:%.*]] = fsub float 0.000000e+00, [[ARG0]]
+; CHECK-NEXT:    ret float [[SUB]]
+;
+  %sub = fsub float 0.0, %arg0
+  ret float %sub
+}
+
+define float @fsub_n0_commute(float %arg0) {
+; CHECK: Function Attrs: nofree norecurse nosync nounwind willreturn memory(none)
+; CHECK-LABEL: define float @fsub_n0_commute
+; CHECK-SAME: (float [[ARG0:%.*]]) #[[ATTR2]] {
+; CHECK-NEXT:    [[SUB:%.*]] = fsub float -0.000000e+00, [[ARG0]]
+; CHECK-NEXT:    ret float [[SUB]]
+;
+  %sub = fsub float -0.0, %arg0
+  ret float %sub
+}