[ValueTracking] teach computeKnownBits that a fabs() clears sign bits

[spatel]

Summary

This was requested in D13076: if we're going to canonicalize to fabs(), ValueTracking should know that fabs() clears sign bits.

In this patch (as in D13076), we're not handling vectors yet even though computeKnownBits' fabs() case itself should be vector-ready via the splat. Fixing this will require follow-on patches that correct logic that uses 'getScalarType'.

[spatel]

Ping.

[majnemer]

LGTM

[arsenm]

Should this also be done in DAGCombiner?

[spatel]

In D13222#263038, @arsenm wrote:

Should this also be done in DAGCombiner?

Sure - I would think that anything that's being done in ValueTracking should be done there too. Can this be refactored, so we're not duplicating code?

[arsenm]

In D13222#263109, @spatel wrote:

In D13222#263038, @arsenm wrote:

Should this also be done in DAGCombiner?

Sure - I would think that anything that's being done in ValueTracking should be done there too. Can this be refactored, so we're not duplicating code?

Not really. The IR and DAG APIs are pretty different. All of this similar logic is already duplicated

lib/Analysis/ValueTracking.cpp

case Instruction::Trunc: {
     KnownOne = KnownOne.zextOrTrunc(BitWidth);
     // Any top bits are known to be zero.
     if (BitWidth > SrcBitWidth)
       KnownZero |= APInt::getHighBitsSet(BitWidth, BitWidth - SrcBitWidth);
     break;
   }
   case Instruction::BitCast: {
     Type *SrcTy = I->getOperand(0)->getType();
-    if ((SrcTy->isIntegerTy() || SrcTy->isPointerTy()) &&
+    if ((SrcTy->isIntegerTy() || SrcTy->isPointerTy() ||
+         SrcTy->isFloatingPointTy()) &&
         // TODO: For now, not handling conversions like:
         // (bitcast i64 %x to <2 x i32>)
         !I->getType()->isVectorTy()) {
       computeKnownBits(I->getOperand(0), KnownZero, KnownOne, DL, Depth + 1, Q);
       break;
     }
     break;
   }
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
         KnownZero |= APInt::getHighBitsSet(BitWidth, BitWidth - LowBits);
         break;
       }
       case Intrinsic::ctpop: {
         unsigned LowBits = Log2_32(BitWidth)+1;
         KnownZero |= APInt::getHighBitsSet(BitWidth, BitWidth - LowBits);
         break;
       }
+      case Intrinsic::fabs: {
+        Type *Ty = II->getType();
+        APInt SignBit = APInt::getSignBit(Ty->getScalarSizeInBits());
+        KnownZero |= APInt::getSplat(Ty->getPrimitiveSizeInBits(), SignBit);
+        break;
+      }
       case Intrinsic::x86_sse42_crc32_64_64:
         KnownZero |= APInt::getHighBitsSet(64, 32);
         break;
       }
     }
     break;
   case Instruction::ExtractValue:
     if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I->getOperand(0))) {

 /// for all of the elements in the vector.
 void computeKnownBits(Value *V, APInt &KnownZero, APInt &KnownOne,
                       const DataLayout &DL, unsigned Depth, const Query &Q) {
   assert(V && "No Value?");
   assert(Depth <= MaxDepth && "Limit Search Depth");
   unsigned BitWidth = KnownZero.getBitWidth();
 
   assert((V->getType()->isIntOrIntVectorTy() ||
+          V->getType()->isFPOrFPVectorTy() ||
           V->getType()->getScalarType()->isPointerTy()) &&
-         "Not integer or pointer type!");
+         "Not integer, floating point, or pointer type!");
   assert((DL.getTypeSizeInBits(V->getType()->getScalarType()) == BitWidth) &&
          (!V->getType()->isIntOrIntVectorTy() ||
           V->getType()->getScalarSizeInBits() == BitWidth) &&
          KnownZero.getBitWidth() == BitWidth &&
          KnownOne.getBitWidth() == BitWidth &&
          "V, KnownOne and KnownZero should have same BitWidth");
 
   if (ConstantInt *CI = dyn_cast<ConstantInt>(V)) {

test/Transforms/InstCombine/fabs.ll

 ; CHECK-NEXT: ret fp128 %mul
 }
 
 ; Make sure all intrinsic calls are eliminated when the input is known positive.
 
 declare float @llvm.fabs.f32(float)
 declare double @llvm.fabs.f64(double)
 declare fp128 @llvm.fabs.f128(fp128)
+declare <4 x float> @llvm.fabs.v4f32(<4 x float>)
 
 define float @square_fabs_intrinsic_f32(float %x) {
   %mul = fmul float %x, %x
   %fabsf = tail call float @llvm.fabs.f32(float %mul)
   ret float %fabsf
 
 ; CHECK-LABEL: square_fabs_intrinsic_f32(
 ; CHECK-NEXT: %mul = fmul float %x, %x
define float @square_fabs_shrink_call2(float %x) {
   %trunc = fptrunc double %fabs to float
   ret float %trunc
 
 ; CHECK-LABEL: square_fabs_shrink_call2(
 ; CHECK-NEXT: %sq = fmul float %x, %x
 ; CHECK-NEXT: ret float %sq
 }
 
+; A scalar fabs op makes the sign bit zero, so masking off all of the other bits means we can return zero.
+
+define i32 @fabs_value_tracking_f32(float %x) {
+  %call = call float @llvm.fabs.f32(float %x)
+  %bc = bitcast float %call to i32
+  %and = and i32 %bc, 2147483648
+  ret i32 %and
+
+; CHECK-LABEL: fabs_value_tracking_f32(
+; CHECK:       ret i32 0
+}
+
+; TODO: A vector fabs op makes the sign bits zero, so masking off all of the other bits means we can return zero.
+
+define <4 x i32> @fabs_value_tracking_v4f32(<4 x float> %x) {
+  %call = call <4 x float> @llvm.fabs.v4f32(<4 x float> %x)
+  %bc = bitcast <4 x float> %call to <4 x i32>
+  %and = and <4 x i32> %bc, <i32 2147483648, i32 2147483648, i32 2147483648, i32 2147483648>
+  ret <4 x i32> %and
+
+; CHECK-LABEL: fabs_value_tracking_v4f32(
+; CHECK:       ret <4 x i32> %and
+}
+