[llvm] 2b6b8cb - [APFloat] Prevent construction of APFloat with Semantics and FP value
Ehud Katz via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 4 02:02:34 PST 2019
Author: Ehud Katz
Date: 2019-12-04T12:02:04+02:00
New Revision: 2b6b8cb10c870d64f7cc29d21fc27cef3c7e0056
URL: https://github.com/llvm/llvm-project/commit/2b6b8cb10c870d64f7cc29d21fc27cef3c7e0056
DIFF: https://github.com/llvm/llvm-project/commit/2b6b8cb10c870d64f7cc29d21fc27cef3c7e0056.diff
LOG: [APFloat] Prevent construction of APFloat with Semantics and FP value
Constructor invocations such as `APFloat(APFloat::IEEEdouble(), 0.0)`
may seem like they accept a FP (floating point) value, but the overload
they reach is actually the `integerPart` one, not a `float` or `double`
overload (which only exists when `fltSemantics` isn't passed).
This may lead to possible loss of data, by the conversion from `float`
or `double` to `integerPart`.
To prevent future mistakes, a new constructor overload, which accepts
any FP value and marked with `delete`, to prevent its usage.
Fixes PR34095.
Differential Revision: https://reviews.llvm.org/D70425
Added:
Modified:
llvm/include/llvm/ADT/APFloat.h
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
llvm/unittests/ADT/APFloatTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h
index 70fc19e82b3c..afeed67e3f9e 100644
--- a/llvm/include/llvm/ADT/APFloat.h
+++ b/llvm/include/llvm/ADT/APFloat.h
@@ -851,6 +851,9 @@ class APFloat : public APFloatBase {
APFloat(const fltSemantics &Semantics) : U(Semantics) {}
APFloat(const fltSemantics &Semantics, StringRef S);
APFloat(const fltSemantics &Semantics, integerPart I) : U(Semantics, I) {}
+ template <typename T, typename = typename std::enable_if<
+ std::is_floating_point<T>::value>::type>
+ APFloat(const fltSemantics &Semantics, T V) = delete;
// TODO: Remove this constructor. This isn't faster than the first one.
APFloat(const fltSemantics &Semantics, uninitializedTag)
: U(Semantics, uninitialized) {}
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 157885e03103..92e9a8814f8f 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3386,7 +3386,7 @@ Instruction *InstCombiner::visitCallInst(CallInst &CI) {
if (const ConstantFP *C = dyn_cast<ConstantFP>(Src)) {
const APFloat &ArgVal = C->getValueAPF();
- APFloat Val(ArgVal.getSemantics(), 1.0);
+ APFloat Val(ArgVal.getSemantics(), 1);
APFloat::opStatus Status = Val.divide(ArgVal,
APFloat::rmNearestTiesToEven);
// Only do this if it was exact and therefore not dependent on the
diff --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
index ef2af01f8af6..44513b1f6827 100644
--- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -1735,7 +1735,7 @@ Value *LibCallSimplifier::optimizePow(CallInst *Pow, IRBuilder<> &B) {
// TODO: This whole transformation should be backend specific (e.g. some
// backends might prefer libcalls or the limit for the exponent might
// be
diff erent) and it should also consider optimizing for size.
- APFloat LimF(ExpoF->getSemantics(), 33.0),
+ APFloat LimF(ExpoF->getSemantics(), 33),
ExpoA(abs(*ExpoF));
if (ExpoA.compare(LimF) == APFloat::cmpLessThan) {
// This transformation applies to integer or integer+0.5 exponents only.
diff --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp
index 656945b325e0..518b207e23e7 100644
--- a/llvm/unittests/ADT/APFloatTest.cpp
+++ b/llvm/unittests/ADT/APFloatTest.cpp
@@ -520,9 +520,9 @@ TEST(APFloatTest, FMA) {
// Test x87 extended precision case from http://llvm.org/PR20728.
{
- APFloat M1(APFloat::x87DoubleExtended(), 1.0);
- APFloat M2(APFloat::x87DoubleExtended(), 1.0);
- APFloat A(APFloat::x87DoubleExtended(), 3.0);
+ APFloat M1(APFloat::x87DoubleExtended(), 1);
+ APFloat M2(APFloat::x87DoubleExtended(), 1);
+ APFloat A(APFloat::x87DoubleExtended(), 3);
bool losesInfo = false;
M1.fusedMultiplyAdd(M1, A, APFloat::rmNearestTiesToEven);
@@ -600,9 +600,9 @@ TEST(APFloatTest, Denormal) {
{
const char *MinNormalStr = "1.17549435082228750797e-38";
EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), MinNormalStr).isDenormal());
- EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), 0.0).isDenormal());
+ EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), 0).isDenormal());
- APFloat Val2(APFloat::IEEEsingle(), 2.0e0);
+ APFloat Val2(APFloat::IEEEsingle(), 2);
APFloat T(APFloat::IEEEsingle(), MinNormalStr);
T.divide(Val2, rdmd);
EXPECT_TRUE(T.isDenormal());
@@ -612,9 +612,9 @@ TEST(APFloatTest, Denormal) {
{
const char *MinNormalStr = "2.22507385850720138309e-308";
EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), MinNormalStr).isDenormal());
- EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), 0.0).isDenormal());
+ EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), 0).isDenormal());
- APFloat Val2(APFloat::IEEEdouble(), 2.0e0);
+ APFloat Val2(APFloat::IEEEdouble(), 2);
APFloat T(APFloat::IEEEdouble(), MinNormalStr);
T.divide(Val2, rdmd);
EXPECT_TRUE(T.isDenormal());
@@ -624,9 +624,9 @@ TEST(APFloatTest, Denormal) {
{
const char *MinNormalStr = "3.36210314311209350626e-4932";
EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), MinNormalStr).isDenormal());
- EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), 0.0).isDenormal());
+ EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), 0).isDenormal());
- APFloat Val2(APFloat::x87DoubleExtended(), 2.0e0);
+ APFloat Val2(APFloat::x87DoubleExtended(), 2);
APFloat T(APFloat::x87DoubleExtended(), MinNormalStr);
T.divide(Val2, rdmd);
EXPECT_TRUE(T.isDenormal());
@@ -636,9 +636,9 @@ TEST(APFloatTest, Denormal) {
{
const char *MinNormalStr = "3.36210314311209350626267781732175260e-4932";
EXPECT_FALSE(APFloat(APFloat::IEEEquad(), MinNormalStr).isDenormal());
- EXPECT_FALSE(APFloat(APFloat::IEEEquad(), 0.0).isDenormal());
+ EXPECT_FALSE(APFloat(APFloat::IEEEquad(), 0).isDenormal());
- APFloat Val2(APFloat::IEEEquad(), 2.0e0);
+ APFloat Val2(APFloat::IEEEquad(), 2);
APFloat T(APFloat::IEEEquad(), MinNormalStr);
T.divide(Val2, rdmd);
EXPECT_TRUE(T.isDenormal());
@@ -1153,8 +1153,8 @@ TEST(APFloatTest, makeNaN) {
#ifdef GTEST_HAS_DEATH_TEST
#ifndef NDEBUG
TEST(APFloatTest, SemanticsDeath) {
- EXPECT_DEATH(APFloat(APFloat::IEEEsingle(), 0.0f).convertToDouble(), "Float semantics are not IEEEdouble");
- EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), 0.0 ).convertToFloat(), "Float semantics are not IEEEsingle");
+ EXPECT_DEATH(APFloat(APFloat::IEEEsingle(), 0).convertToDouble(), "Float semantics are not IEEEdouble");
+ EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), 0).convertToFloat(), "Float semantics are not IEEEsingle");
}
TEST(APFloatTest, StringDecimalDeath) {
More information about the llvm-commits
mailing list