[prev in list] [next in list] [prev in thread] [next in thread]
List: llvm-commits
Subject: Re: [PATCH] D19178: Broaden FoldItoFPtoI to try and establish whether the integer value fits into th
From: Carlos Liam via llvm-commits <llvm-commits () lists ! llvm ! org>
Date: 2016-07-31 23:51:36
Message-ID: f301bb8e9c0eca6969e81e4de63c5059 () localhost ! localdomain
[Download RAW message or body]
aarzee updated this revision to Diff 66257.
aarzee added a comment.
Modified most and least significant possibly set bit values to act more lik=
e range bounds; modified and added unknown sign tests.
https://reviews.llvm.org/D19178
Files:
lib/Transforms/InstCombine/InstCombineCasts.cpp
test/Transforms/InstCombine/sitofp.ll
["D19178.66257.patch" (text/x-patch)]
Index: test/Transforms/InstCombine/sitofp.ll
===================================================================
--- test/Transforms/InstCombine/sitofp.ll
+++ test/Transforms/InstCombine/sitofp.ll
@@ -182,3 +182,132 @@
ret i55 %C
}
+; This should fold because even though the bit width of the integer
+; is greater than that of the mantissa, we can establish that the
+; actual value of the integer will fit into the mantissa.
+; (Bits of %B: 00000000UUUUUUUUUUUUUUUUUUUUUUUU, U = unknown)
+; CHECK-LABEL: test20
+; CHECK: and
+; CHECK-NEXT: ret i32
+define i32 @test20(i32 %A) nounwind {
+ %B = and i32 %A, 16777215
+ %C = sitofp i32 %B to float
+ %D = fptosi float %C to i32
+ ret i32 %D
+}
+
+; Same as last test, but unsigned.
+; CHECK-LABEL: test21
+; CHECK: and
+; CHECK-NEXT: ret i32
+define i32 @test21(i32 %A) nounwind {
+ %B = and i32 %A, 16777215
+ %C = uitofp i32 %B to float
+ %D = fptoui float %C to i32
+ ret i32 %D
+}
+
+; This should not fold, because the integer's value does not fit
+; into the significand.
+; (Bits of %B: 0000000UUUUUUUUUUUUUUUUUUUUUUUUU, U = unknown)
+; CHECK-LABEL: test22
+; CHECK: sitofp
+; CHECK-NEXT: fptosi
+define i32 @test22(i32 %A) nounwind {
+ %B = and i32 %A, 33554431
+ %C = sitofp i32 %B to float
+ %D = fptosi float %C to i32
+ ret i32 %D
+}
+
+; Same as last test, but unsigned.
+; CHECK-LABEL: test23
+; CHECK: uitofp
+; CHECK-NEXT: fptoui
+define i32 @test23(i32 %A) nounwind {
+ %B = and i32 %A, 33554431
+ %C = uitofp i32 %B to float
+ %D = fptoui float %C to i32
+ ret i32 %D
+}
+
+; This should fold because due to the sign being set, the highest
+; possibly set bit of the absolute value is the bit to the left of
+; the highest not known one bit (in this case, position 0 + 1 = 1).
+; (Bits of %B: 1111111111111111111111111111111U, U = unknown)
+; CHECK-LABEL: test24
+; CHECK: or
+; CHECK-NEXT: ret i32
+define i32 @test24(i32 %A) nounwind {
+ %B = or i32 %A, -2
+ %C = sitofp i32 %B to float
+ %D = fptosi float %C to i32
+ ret i32 %D
+}
+
+; This should not fold because of the same property.
+; (Bits of %B: 10000000UUUUUUUUUUUUUUUUUUUUUUUU, U = unknown)
+; CHECK-LABEL: test25
+; CHECK: sitofp
+; CHECK-NEXT: fptosi
+define i32 @test25(i32 %A) nounwind {
+ %B = and i32 %A, 16777215
+ %C = or i32 -2147483648, %B
+ %D = sitofp i32 %C to float
+ %E = fptosi float %D to i32
+ ret i32 %E
+}
+
+; This should fold because the exponent can shift
+; the number any amount of bits to the left.
+; (Bits of %B: 0000000UUUUUUUUUUUUUUUUUUUUUUUU0, U = unknown)
+; CHECK-LABEL: test26
+; CHECK: and
+; CHECK-NEXT: ret i32
+define i32 @test26(i32 %A) nounwind {
+ %B = and i32 %A, 33554430
+ %C = sitofp i32 %B to float
+ %D = fptosi float %C to i32
+ ret i32 %D
+}
+
+; This should fold because even with unknown sign,
+; the lowest possibly set bit is the lowest not known
+; zero, and the highest possibly set bit is the one to
+; the right of the sign bit.
+; (Bits of %B: UUUUUUUUUUUUUUUUUUUUUUUUU000000000, U = unknown)
+; CHECK-LABEL: test27
+; CHECK: and
+; CHECK-NEXT: ret i32
+define i32 @test27(i32 %A) nounwind {
+ %B = and i32 %A, -128
+ %C = sitofp i32 %B to float
+ %D = fptosi float %C to i32
+ ret i32 %D
+}
+
+; This should not fold for the same reason.
+; (Bits of %B: UUUUUUUUUUUUUUUUUUUUUUUUUU0000000, U = unknown)
+; CHECK-LABEL: test28
+; CHECK: sitofp
+; CHECK-NEXT: fptosi
+define i32 @test28(i32 %A) nounwind {
+ %B = and i32 %A, -64
+ %C = sitofp i32 %B to float
+ %D = fptosi float %C to i32
+ ret i32 %D
+}
+
+; This should fold despite triggering some unusual behavior
+; in the relevant code (the highest possibly set bit value is
+; lower than the lowest possibly set bit value).
+; (Bits of %B: U0000000000000000000000000000000, U = unknown)
+; CHECK-LABEL: test29
+; CHECK: and
+; CHECK-NEXT: ret i32
+define i32 @test29(i32 %A) nounwind {
+ %B = and i32 %A, -2147483648
+ %C = sitofp i32 %B to float
+ %D = fptosi float %C to i32
+ ret i32 %D
+}
Index: lib/Transforms/InstCombine/InstCombineCasts.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -480,13 +480,13 @@
// Test if the trunc is the user of a select which is part of a
// minimum or maximum operation. If so, don't do any more simplification.
- // Even simplifying demanded bits can break the canonical form of a
+ // Even simplifying demanded bits can break the canonical form of a
// min/max.
Value *LHS, *RHS;
if (SelectInst *SI = dyn_cast<SelectInst>(CI.getOperand(0)))
if (matchSelectPattern(SI, LHS, RHS).Flavor != SPF_UNKNOWN)
return nullptr;
-
+
// See if we can simplify any instructions used by the input whose sole
// purpose is to compute bits we don't care about.
if (SimplifyDemandedInstructionBits(CI))
@@ -1132,7 +1132,7 @@
Type *SrcTy = Src->getType(), *DestTy = CI.getType();
// If we know that the value being extended is positive, we can use a zext
- // instead.
+ // instead.
bool KnownZero, KnownOne;
ComputeSignBit(Src, KnownZero, KnownOne, 0, &CI);
if (KnownZero) {
@@ -1416,10 +1416,14 @@
return commonCastTransforms(CI);
}
-// fpto{s/u}i({u/s}itofp(X)) --> X or zext(X) or sext(X) or trunc(X)
-// This is safe if the intermediate type has enough bits in its mantissa to
+// fpto{s/u}i({s/u}itofp(X)) --> X or zext(X) or sext(X) or trunc(X)
+// This is safe if the intermediate type has enough bits in its significand to
// accurately represent all values of X. For example, this won't work with
// i64 -> float -> i64.
+// However, this is also safe if we can establish the range between most and
+// least significant set bits of abs(X) fits into the significand. We don't need
+// to worry about the exponent since converting an integer to an fp type that
+// it cannot fit in is undefined behavior.
Instruction *InstCombiner::FoldItoFPtoI(Instruction &FI) {
if (!isa<UIToFPInst>(FI.getOperand(0)) && !isa<SIToFPInst>(FI.getOperand(0)))
return nullptr;
@@ -1445,7 +1449,65 @@
int OutputSize = (int)FITy->getScalarSizeInBits() - IsOutputSigned;
int ActualSize = std::min(InputSize, OutputSize);
- if (ActualSize <= OpITy->getFPMantissaWidth()) {
+ int SignificandWidth = OpITy->getFPMantissaWidth();
+
+ bool Safe = ActualSize <= SignificandWidth;
+
+ if (!Safe) {
+ // Compute known bits to see if the integer value fits into the fp type.
+
+ uint32_t BitWidth = SrcTy->getScalarSizeInBits();
+ APInt KnownZero(BitWidth, 0), KnownOne(BitWidth, 0);
+ computeKnownBits(SrcI, KnownZero, KnownOne, 0, &FI);
+
+ // MostSignificantPossiblySetBit and LeastSignificantPossiblySetBit refer to
+ // the high and low ends of the range of set bits of the absolute value of
+ // the integer.
+
+ int LeastSignificantPossiblySetBit;
+ int MostSignificantPossiblySetBit;
+
+ bool PossiblyNegative = IsInputSigned && !KnownZero[BitWidth - 1];
+ bool KnownNegative = PossiblyNegative && KnownOne[BitWidth - 1];
+
+ // The lowest possibly set bit is always the lowest bit not known to be
+ // zero.
+
+ LeastSignificantPossiblySetBit = int(KnownZero.countTrailingOnes());
+
+ // If the number is known to be negative, the highest possibly set bit is
+ // the highest bit not known to be one.
+ // If the number is known to be positive, the highest possibly set bit is
+ // the highest bit not known to be zero.
+ // If the number is of unknown sign, the highest possibly set bit is the
+ // bit to the right of the sign bit. This causes an interesting behavior
+ // if the sign bit is unknown and every other bit is known zero: the value
+ // we set for lowest possibly set bit will be 1 higher than that for highest
+ // possibly set bit, leading to a BitRange of 0. The real BitRange would be
+ // 1, which will fit into any significand, so this doesn't cause any
+ // incorrect behavior.
+
+ if (KnownNegative)
+ MostSignificantPossiblySetBit = int(BitWidth -
+ KnownOne.countLeadingOnes());
+ else if (!PossiblyNegative)
+ MostSignificantPossiblySetBit = int(BitWidth -
+ KnownZero.countLeadingOnes());
+ else
+ MostSignificantPossiblySetBit = int(BitWidth) - 1;
+
+ int BitRange = MostSignificantPossiblySetBit -
+ LeastSignificantPossiblySetBit;
+
+ // To be safe, the length of the range between the most and least
+ // significant possibly set bits must fit within the width of the
+ // significand. We don't have to worry about the exponent; if the number
+ // doesn't fit into the exponent, then the conversion is undefined behavior.
+
+ Safe = BitRange <= SignificandWidth;
+ }
+
+ if (Safe) {
if (FITy->getScalarSizeInBits() > SrcTy->getScalarSizeInBits()) {
if (IsInputSigned && IsOutputSigned)
return new SExtInst(SrcI, FITy);
[Attachment #4 (text/plain)]
_______________________________________________
llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic