Enable logf128 constant folding for hosts with 128bit long double #104929
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a reland of (llvm#96287). This patch attempts to reduce clang's compile time by removing #includes of float128.h and inlining convertToQuad functions instead.
llvmbot
added
cmake
|
@llvm/pr-subscribers-llvm-adt @llvm/pr-subscribers-llvm-support Author: Matthew Devereau (MDevereau) ChangesThis is a reland of (#96287). This patch attempts to reduce clang's compile time by removing #includes of float128.h and inlining convertToQuad functions instead. Full diff: https://github.com/llvm/llvm-project/pull/104929.diff 8 Files Affected:
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index d681b1ccab6299..b03d89a43c34b0 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -560,8 +560,6 @@ set(LLVM_USE_STATIC_ZSTD FALSE CACHE BOOL "Use static version of zstd. Can be TR
set(LLVM_ENABLE_CURL "OFF" CACHE STRING "Use libcurl for the HTTP client if available. Can be ON, OFF, or FORCE_ON")
-set(LLVM_HAS_LOGF128 "OFF" CACHE STRING "Use logf128 to constant fold fp128 logarithm calls. Can be ON, OFF, or FORCE_ON")
-
set(LLVM_ENABLE_HTTPLIB "OFF" CACHE STRING "Use cpp-httplib HTTP server library if available. Can be ON, OFF, or FORCE_ON")
set(LLVM_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the Z3 solver.")
diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake
index 0aae13e30f2ab4..976213bb9e948a 100644
--- a/llvm/cmake/config-ix.cmake
+++ b/llvm/cmake/config-ix.cmake
@@ -247,17 +247,6 @@ else()
set(HAVE_LIBEDIT 0)
endif()
-if(LLVM_HAS_LOGF128)
- include(CheckCXXSymbolExists)
- check_cxx_symbol_exists(logf128 math.h HAS_LOGF128)
-
- if(LLVM_HAS_LOGF128 STREQUAL FORCE_ON AND NOT HAS_LOGF128)
- message(FATAL_ERROR "Failed to configure logf128")
- endif()
-
- set(LLVM_HAS_LOGF128 "${HAS_LOGF128}")
-endif()
-
# function checks
check_symbol_exists(arc4random "stdlib.h" HAVE_DECL_ARC4RANDOM)
find_package(Backtrace)
@@ -271,6 +260,13 @@ if(C_SUPPORTS_WERROR_UNGUARDED_AVAILABILITY_NEW)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror=unguarded-availability-new")
endif()
+check_cxx_symbol_exists(logf128 cmath HAS_LOGF128)
+check_symbol_exists(__powerpc64le__ "" __PPC64LE)
+if(HAS_LOGF128 AND NOT __PPC64LE)
+ set(LLVM_HAS_LOGF128 On)
+ add_compile_definitions(HAS_LOGF128)
+endif()
+
# Determine whether we can register EH tables.
check_symbol_exists(__register_frame "${CMAKE_CURRENT_LIST_DIR}/unwind.h" HAVE_REGISTER_FRAME)
check_symbol_exists(__deregister_frame "${CMAKE_CURRENT_LIST_DIR}/unwind.h" HAVE_DEREGISTER_FRAME)
diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h
index 7039e961bff82d..925d03d4c06670 100644
--- a/llvm/include/llvm/ADT/APFloat.h
+++ b/llvm/include/llvm/ADT/APFloat.h
@@ -19,7 +19,6 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/FloatingPointMode.h"
#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/float128.h"
#include <memory>
#define APFLOAT_DISPATCH_ON_SEMANTICS(METHOD_CALL) \
@@ -378,9 +377,6 @@ class IEEEFloat final : public APFloatBase {
Expected<opStatus> convertFromString(StringRef, roundingMode);
APInt bitcastToAPInt() const;
double convertToDouble() const;
-#ifdef HAS_IEE754_FLOAT128
- float128 convertToQuad() const;
-#endif
float convertToFloat() const;
/// @}
@@ -1274,14 +1270,9 @@ class APFloat : public APFloatBase {
/// shorter semantics, like IEEEsingle and others.
double convertToDouble() const;
- /// Converts this APFloat to host float value.
- ///
- /// \pre The APFloat must be built using semantics, that can be represented by
- /// the host float type without loss of precision. It can be IEEEquad and
- /// shorter semantics, like IEEEdouble and others.
-#ifdef HAS_IEE754_FLOAT128
- float128 convertToQuad() const;
-#endif
+ /// Return true if this APFloat has quadruple precision floating point
+ /// semantics
+ bool isValidIEEEQuad() const;
/// Converts this APFloat to host float value.
///
diff --git a/llvm/include/llvm/ADT/APInt.h b/llvm/include/llvm/ADT/APInt.h
index 108df7e0eaeaa3..62e41ce88710b1 100644
--- a/llvm/include/llvm/ADT/APInt.h
+++ b/llvm/include/llvm/ADT/APInt.h
@@ -17,7 +17,6 @@
#include "llvm/Support/Compiler.h"
#include "llvm/Support/MathExtras.h"
-#include "llvm/Support/float128.h"
#include <cassert>
#include <climits>
#include <cstring>
@@ -1679,13 +1678,6 @@ class [[nodiscard]] APInt {
/// any bit width. Exactly 64 bits will be translated.
double bitsToDouble() const { return llvm::bit_cast<double>(getWord(0)); }
-#ifdef HAS_IEE754_FLOAT128
- float128 bitsToQuad() const {
- __uint128_t ul = ((__uint128_t)U.pVal[1] << 64) + U.pVal[0];
- return llvm::bit_cast<float128>(ul);
- }
-#endif
-
/// Converts APInt bits to a float
///
/// The conversion does not do a translation from integer to float, it just
@@ -1883,6 +1875,12 @@ class [[nodiscard]] APInt {
/// Returns whether this instance allocated memory.
bool needsCleanup() const { return !isSingleWord(); }
+ /// Get the word corresponding to a bit position
+ /// \returns the corresponding word for the specified bit position.
+ uint64_t getWord(unsigned bitPosition) const {
+ return isSingleWord() ? U.VAL : U.pVal[whichWord(bitPosition)];
+ }
+
private:
/// This union is used to store the integer value. When the
/// integer bit-width <= 64, it uses VAL, otherwise it uses pVal.
@@ -1948,12 +1946,6 @@ class [[nodiscard]] APInt {
return *this;
}
- /// Get the word corresponding to a bit position
- /// \returns the corresponding word for the specified bit position.
- uint64_t getWord(unsigned bitPosition) const {
- return isSingleWord() ? U.VAL : U.pVal[whichWord(bitPosition)];
- }
-
/// Utility method to change the bit width of this APInt to new bit width,
/// allocating and/or deallocating as necessary. There is no guarantee on the
/// value of any bits upon return. Caller should populate the bits after.
diff --git a/llvm/include/llvm/Support/float128.h b/llvm/include/llvm/Support/float128.h
index e15a98dc5a6779..618b320086ba59 100644
--- a/llvm/include/llvm/Support/float128.h
+++ b/llvm/include/llvm/Support/float128.h
@@ -9,18 +9,16 @@
#ifndef LLVM_FLOAT128
#define LLVM_FLOAT128
+#include <cmath>
+
namespace llvm {
-#if defined(__clang__) && defined(__FLOAT128__) && \
- defined(__SIZEOF_INT128__) && !defined(__LONG_DOUBLE_IBM128__)
-#define HAS_IEE754_FLOAT128
-typedef __float128 float128;
-#elif defined(__FLOAT128__) && defined(__SIZEOF_INT128__) && \
- !defined(__LONG_DOUBLE_IBM128__) && \
- (defined(__GNUC__) || defined(__GNUG__))
+#ifdef HAS_LOGF128
+#if !defined(__LONG_DOUBLE_IBM128__) && (__SIZEOF_INT128__ == 16)
+typedef decltype(logf128(0.)) float128;
#define HAS_IEE754_FLOAT128
-typedef _Float128 float128;
#endif
+#endif // HAS_LOGF128
} // namespace llvm
#endif // LLVM_FLOAT128
diff --git a/llvm/lib/Analysis/CMakeLists.txt b/llvm/lib/Analysis/CMakeLists.txt
index 393803fad89383..3127f45cc54cb1 100644
--- a/llvm/lib/Analysis/CMakeLists.txt
+++ b/llvm/lib/Analysis/CMakeLists.txt
@@ -162,9 +162,3 @@ add_llvm_component_library(LLVMAnalysis
Support
TargetParser
)
-
-include(CheckCXXSymbolExists)
-check_cxx_symbol_exists(logf128 math.h HAS_LOGF128)
-if(HAS_LOGF128)
- target_compile_definitions(LLVMAnalysis PRIVATE HAS_LOGF128)
-endif()
diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index defcacdfa8b105..b3bea0722f7530 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -54,6 +54,7 @@
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/KnownBits.h"
#include "llvm/Support/MathExtras.h"
+#include "llvm/Support/float128.h"
#include <cassert>
#include <cerrno>
#include <cfenv>
@@ -1741,7 +1742,7 @@ Constant *GetConstantFoldFPValue(double V, Type *Ty) {
llvm_unreachable("Can only constant fold half/float/double");
}
-#if defined(HAS_IEE754_FLOAT128) && defined(HAS_LOGF128)
+#if defined(HAS_IEE754_FLOAT128)
Constant *GetConstantFoldFPValue128(float128 V, Type *Ty) {
if (Ty->isFP128Ty())
return ConstantFP::get(Ty, V);
@@ -1781,11 +1782,18 @@ Constant *ConstantFoldFP(double (*NativeFP)(double), const APFloat &V,
return GetConstantFoldFPValue(Result, Ty);
}
-#if defined(HAS_IEE754_FLOAT128) && defined(HAS_LOGF128)
+#if defined(HAS_IEE754_FLOAT128)
Constant *ConstantFoldFP128(float128 (*NativeFP)(float128), const APFloat &V,
Type *Ty) {
llvm_fenv_clearexcept();
- float128 Result = NativeFP(V.convertToQuad());
+
+ if (!V.isValidIEEEQuad())
+ return nullptr;
+
+ APInt Api = V.bitcastToAPInt();
+ __uint128_t Int128 = ((__uint128_t)Api.getWord(64) << 64) + Api.getWord(0);
+ float128 Result = NativeFP(llvm::bit_cast<float128>(Int128));
+
if (llvm_fenv_testexcept()) {
llvm_fenv_clearexcept();
return nullptr;
@@ -2114,10 +2122,16 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
if (IntrinsicID == Intrinsic::canonicalize)
return constantFoldCanonicalize(Ty, Call, U);
-#if defined(HAS_IEE754_FLOAT128) && defined(HAS_LOGF128)
+#if defined(HAS_IEE754_FLOAT128)
if (Ty->isFP128Ty()) {
if (IntrinsicID == Intrinsic::log) {
- float128 Result = logf128(Op->getValueAPF().convertToQuad());
+ APFloat Value = Op->getValueAPF();
+ if (!Value.isValidIEEEQuad())
+ return nullptr;
+ APInt api = Value.bitcastToAPInt();
+ __uint128_t Int128 =
+ ((__uint128_t)api.getWord(64) << 64) + api.getWord(0);
+ float128 Result = logf128(llvm::bit_cast<float128>(Int128));
return GetConstantFoldFPValue128(Result, Ty);
}
diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 7f68c5ab9b7cf7..2ddf99f56f88d5 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -3749,15 +3749,6 @@ double IEEEFloat::convertToDouble() const {
return api.bitsToDouble();
}
-#ifdef HAS_IEE754_FLOAT128
-float128 IEEEFloat::convertToQuad() const {
- assert(semantics == (const llvm::fltSemantics *)&semIEEEquad &&
- "Float semantics are not IEEEquads");
- APInt api = bitcastToAPInt();
- return api.bitsToQuad();
-}
-#endif
-
/// Integer bit is explicit in this format. Intel hardware (387 and later)
/// does not support these bit patterns:
/// exponent = all 1's, integer bit 0, significand 0 ("pseudoinfinity")
@@ -5406,20 +5397,9 @@ double APFloat::convertToDouble() const {
return Temp.getIEEE().convertToDouble();
}
-#ifdef HAS_IEE754_FLOAT128
-float128 APFloat::convertToQuad() const {
- if (&getSemantics() == (const llvm::fltSemantics *)&semIEEEquad)
- return getIEEE().convertToQuad();
- assert(getSemantics().isRepresentableBy(semIEEEquad) &&
- "Float semantics is not representable by IEEEquad");
- APFloat Temp = *this;
- bool LosesInfo;
- opStatus St = Temp.convert(semIEEEquad, rmNearestTiesToEven, &LosesInfo);
- assert(!(St & opInexact) && !LosesInfo && "Unexpected imprecision");
- (void)St;
- return Temp.getIEEE().convertToQuad();
+bool APFloat::isValidIEEEQuad() const {
+ return (&getSemantics() == (const llvm::fltSemantics *)&semIEEEquad);
}
-#endif
float APFloat::convertToFloat() const {
if (&getSemantics() == (const llvm::fltSemantics *)&semIEEEsingle)
|
|
@llvm/pr-subscribers-llvm-analysis Author: Matthew Devereau (MDevereau) ChangesThis is a reland of (#96287). This patch attempts to reduce clang's compile time by removing #includes of float128.h and inlining convertToQuad functions instead. Full diff: https://github.com/llvm/llvm-project/pull/104929.diff 8 Files Affected:
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index d681b1ccab6299..b03d89a43c34b0 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -560,8 +560,6 @@ set(LLVM_USE_STATIC_ZSTD FALSE CACHE BOOL "Use static version of zstd. Can be TR
set(LLVM_ENABLE_CURL "OFF" CACHE STRING "Use libcurl for the HTTP client if available. Can be ON, OFF, or FORCE_ON")
-set(LLVM_HAS_LOGF128 "OFF" CACHE STRING "Use logf128 to constant fold fp128 logarithm calls. Can be ON, OFF, or FORCE_ON")
-
set(LLVM_ENABLE_HTTPLIB "OFF" CACHE STRING "Use cpp-httplib HTTP server library if available. Can be ON, OFF, or FORCE_ON")
set(LLVM_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the Z3 solver.")
diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake
index 0aae13e30f2ab4..976213bb9e948a 100644
--- a/llvm/cmake/config-ix.cmake
+++ b/llvm/cmake/config-ix.cmake
@@ -247,17 +247,6 @@ else()
set(HAVE_LIBEDIT 0)
endif()
-if(LLVM_HAS_LOGF128)
- include(CheckCXXSymbolExists)
- check_cxx_symbol_exists(logf128 math.h HAS_LOGF128)
-
- if(LLVM_HAS_LOGF128 STREQUAL FORCE_ON AND NOT HAS_LOGF128)
- message(FATAL_ERROR "Failed to configure logf128")
- endif()
-
- set(LLVM_HAS_LOGF128 "${HAS_LOGF128}")
-endif()
-
# function checks
check_symbol_exists(arc4random "stdlib.h" HAVE_DECL_ARC4RANDOM)
find_package(Backtrace)
@@ -271,6 +260,13 @@ if(C_SUPPORTS_WERROR_UNGUARDED_AVAILABILITY_NEW)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror=unguarded-availability-new")
endif()
+check_cxx_symbol_exists(logf128 cmath HAS_LOGF128)
+check_symbol_exists(__powerpc64le__ "" __PPC64LE)
+if(HAS_LOGF128 AND NOT __PPC64LE)
+ set(LLVM_HAS_LOGF128 On)
+ add_compile_definitions(HAS_LOGF128)
+endif()
+
# Determine whether we can register EH tables.
check_symbol_exists(__register_frame "${CMAKE_CURRENT_LIST_DIR}/unwind.h" HAVE_REGISTER_FRAME)
check_symbol_exists(__deregister_frame "${CMAKE_CURRENT_LIST_DIR}/unwind.h" HAVE_DEREGISTER_FRAME)
diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h
index 7039e961bff82d..925d03d4c06670 100644
--- a/llvm/include/llvm/ADT/APFloat.h
+++ b/llvm/include/llvm/ADT/APFloat.h
@@ -19,7 +19,6 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/FloatingPointMode.h"
#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/float128.h"
#include <memory>
#define APFLOAT_DISPATCH_ON_SEMANTICS(METHOD_CALL) \
@@ -378,9 +377,6 @@ class IEEEFloat final : public APFloatBase {
Expected<opStatus> convertFromString(StringRef, roundingMode);
APInt bitcastToAPInt() const;
double convertToDouble() const;
-#ifdef HAS_IEE754_FLOAT128
- float128 convertToQuad() const;
-#endif
float convertToFloat() const;
/// @}
@@ -1274,14 +1270,9 @@ class APFloat : public APFloatBase {
/// shorter semantics, like IEEEsingle and others.
double convertToDouble() const;
- /// Converts this APFloat to host float value.
- ///
- /// \pre The APFloat must be built using semantics, that can be represented by
- /// the host float type without loss of precision. It can be IEEEquad and
- /// shorter semantics, like IEEEdouble and others.
-#ifdef HAS_IEE754_FLOAT128
- float128 convertToQuad() const;
-#endif
+ /// Return true if this APFloat has quadruple precision floating point
+ /// semantics
+ bool isValidIEEEQuad() const;
/// Converts this APFloat to host float value.
///
diff --git a/llvm/include/llvm/ADT/APInt.h b/llvm/include/llvm/ADT/APInt.h
index 108df7e0eaeaa3..62e41ce88710b1 100644
--- a/llvm/include/llvm/ADT/APInt.h
+++ b/llvm/include/llvm/ADT/APInt.h
@@ -17,7 +17,6 @@
#include "llvm/Support/Compiler.h"
#include "llvm/Support/MathExtras.h"
-#include "llvm/Support/float128.h"
#include <cassert>
#include <climits>
#include <cstring>
@@ -1679,13 +1678,6 @@ class [[nodiscard]] APInt {
/// any bit width. Exactly 64 bits will be translated.
double bitsToDouble() const { return llvm::bit_cast<double>(getWord(0)); }
-#ifdef HAS_IEE754_FLOAT128
- float128 bitsToQuad() const {
- __uint128_t ul = ((__uint128_t)U.pVal[1] << 64) + U.pVal[0];
- return llvm::bit_cast<float128>(ul);
- }
-#endif
-
/// Converts APInt bits to a float
///
/// The conversion does not do a translation from integer to float, it just
@@ -1883,6 +1875,12 @@ class [[nodiscard]] APInt {
/// Returns whether this instance allocated memory.
bool needsCleanup() const { return !isSingleWord(); }
+ /// Get the word corresponding to a bit position
+ /// \returns the corresponding word for the specified bit position.
+ uint64_t getWord(unsigned bitPosition) const {
+ return isSingleWord() ? U.VAL : U.pVal[whichWord(bitPosition)];
+ }
+
private:
/// This union is used to store the integer value. When the
/// integer bit-width <= 64, it uses VAL, otherwise it uses pVal.
@@ -1948,12 +1946,6 @@ class [[nodiscard]] APInt {
return *this;
}
- /// Get the word corresponding to a bit position
- /// \returns the corresponding word for the specified bit position.
- uint64_t getWord(unsigned bitPosition) const {
- return isSingleWord() ? U.VAL : U.pVal[whichWord(bitPosition)];
- }
-
/// Utility method to change the bit width of this APInt to new bit width,
/// allocating and/or deallocating as necessary. There is no guarantee on the
/// value of any bits upon return. Caller should populate the bits after.
diff --git a/llvm/include/llvm/Support/float128.h b/llvm/include/llvm/Support/float128.h
index e15a98dc5a6779..618b320086ba59 100644
--- a/llvm/include/llvm/Support/float128.h
+++ b/llvm/include/llvm/Support/float128.h
@@ -9,18 +9,16 @@
#ifndef LLVM_FLOAT128
#define LLVM_FLOAT128
+#include <cmath>
+
namespace llvm {
-#if defined(__clang__) && defined(__FLOAT128__) && \
- defined(__SIZEOF_INT128__) && !defined(__LONG_DOUBLE_IBM128__)
-#define HAS_IEE754_FLOAT128
-typedef __float128 float128;
-#elif defined(__FLOAT128__) && defined(__SIZEOF_INT128__) && \
- !defined(__LONG_DOUBLE_IBM128__) && \
- (defined(__GNUC__) || defined(__GNUG__))
+#ifdef HAS_LOGF128
+#if !defined(__LONG_DOUBLE_IBM128__) && (__SIZEOF_INT128__ == 16)
+typedef decltype(logf128(0.)) float128;
#define HAS_IEE754_FLOAT128
-typedef _Float128 float128;
#endif
+#endif // HAS_LOGF128
} // namespace llvm
#endif // LLVM_FLOAT128
diff --git a/llvm/lib/Analysis/CMakeLists.txt b/llvm/lib/Analysis/CMakeLists.txt
index 393803fad89383..3127f45cc54cb1 100644
--- a/llvm/lib/Analysis/CMakeLists.txt
+++ b/llvm/lib/Analysis/CMakeLists.txt
@@ -162,9 +162,3 @@ add_llvm_component_library(LLVMAnalysis
Support
TargetParser
)
-
-include(CheckCXXSymbolExists)
-check_cxx_symbol_exists(logf128 math.h HAS_LOGF128)
-if(HAS_LOGF128)
- target_compile_definitions(LLVMAnalysis PRIVATE HAS_LOGF128)
-endif()
diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index defcacdfa8b105..b3bea0722f7530 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -54,6 +54,7 @@
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/KnownBits.h"
#include "llvm/Support/MathExtras.h"
+#include "llvm/Support/float128.h"
#include <cassert>
#include <cerrno>
#include <cfenv>
@@ -1741,7 +1742,7 @@ Constant *GetConstantFoldFPValue(double V, Type *Ty) {
llvm_unreachable("Can only constant fold half/float/double");
}
-#if defined(HAS_IEE754_FLOAT128) && defined(HAS_LOGF128)
+#if defined(HAS_IEE754_FLOAT128)
Constant *GetConstantFoldFPValue128(float128 V, Type *Ty) {
if (Ty->isFP128Ty())
return ConstantFP::get(Ty, V);
@@ -1781,11 +1782,18 @@ Constant *ConstantFoldFP(double (*NativeFP)(double), const APFloat &V,
return GetConstantFoldFPValue(Result, Ty);
}
-#if defined(HAS_IEE754_FLOAT128) && defined(HAS_LOGF128)
+#if defined(HAS_IEE754_FLOAT128)
Constant *ConstantFoldFP128(float128 (*NativeFP)(float128), const APFloat &V,
Type *Ty) {
llvm_fenv_clearexcept();
- float128 Result = NativeFP(V.convertToQuad());
+
+ if (!V.isValidIEEEQuad())
+ return nullptr;
+
+ APInt Api = V.bitcastToAPInt();
+ __uint128_t Int128 = ((__uint128_t)Api.getWord(64) << 64) + Api.getWord(0);
+ float128 Result = NativeFP(llvm::bit_cast<float128>(Int128));
+
if (llvm_fenv_testexcept()) {
llvm_fenv_clearexcept();
return nullptr;
@@ -2114,10 +2122,16 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
if (IntrinsicID == Intrinsic::canonicalize)
return constantFoldCanonicalize(Ty, Call, U);
-#if defined(HAS_IEE754_FLOAT128) && defined(HAS_LOGF128)
+#if defined(HAS_IEE754_FLOAT128)
if (Ty->isFP128Ty()) {
if (IntrinsicID == Intrinsic::log) {
- float128 Result = logf128(Op->getValueAPF().convertToQuad());
+ APFloat Value = Op->getValueAPF();
+ if (!Value.isValidIEEEQuad())
+ return nullptr;
+ APInt api = Value.bitcastToAPInt();
+ __uint128_t Int128 =
+ ((__uint128_t)api.getWord(64) << 64) + api.getWord(0);
+ float128 Result = logf128(llvm::bit_cast<float128>(Int128));
return GetConstantFoldFPValue128(Result, Ty);
}
diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 7f68c5ab9b7cf7..2ddf99f56f88d5 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -3749,15 +3749,6 @@ double IEEEFloat::convertToDouble() const {
return api.bitsToDouble();
}
-#ifdef HAS_IEE754_FLOAT128
-float128 IEEEFloat::convertToQuad() const {
- assert(semantics == (const llvm::fltSemantics *)&semIEEEquad &&
- "Float semantics are not IEEEquads");
- APInt api = bitcastToAPInt();
- return api.bitsToQuad();
-}
-#endif
-
/// Integer bit is explicit in this format. Intel hardware (387 and later)
/// does not support these bit patterns:
/// exponent = all 1's, integer bit 0, significand 0 ("pseudoinfinity")
@@ -5406,20 +5397,9 @@ double APFloat::convertToDouble() const {
return Temp.getIEEE().convertToDouble();
}
-#ifdef HAS_IEE754_FLOAT128
-float128 APFloat::convertToQuad() const {
- if (&getSemantics() == (const llvm::fltSemantics *)&semIEEEquad)
- return getIEEE().convertToQuad();
- assert(getSemantics().isRepresentableBy(semIEEEquad) &&
- "Float semantics is not representable by IEEEquad");
- APFloat Temp = *this;
- bool LosesInfo;
- opStatus St = Temp.convert(semIEEEquad, rmNearestTiesToEven, &LosesInfo);
- assert(!(St & opInexact) && !LosesInfo && "Unexpected imprecision");
- (void)St;
- return Temp.getIEEE().convertToQuad();
+bool APFloat::isValidIEEEQuad() const {
+ return (&getSemantics() == (const llvm::fltSemantics *)&semIEEEquad);
}
-#endif
float APFloat::convertToFloat() const {
if (&getSemantics() == (const llvm::fltSemantics *)&semIEEEsingle)
|
davemgreen
reviewed
Aug 20, 2024
llvm/include/llvm/ADT/APInt.h
Outdated
| @@ -1883,6 +1875,12 @@ class [[nodiscard]] APInt { | |||
| /// Returns whether this instance allocated memory. | |||
| bool needsCleanup() const { return !isSingleWord(); } | |||
|
|
|||
| /// Get the word corresponding to a bit position | |||
| /// \returns the corresponding word for the specified bit position. | |||
| uint64_t getWord(unsigned bitPosition) const { | |||
There was a problem hiding this comment.
Would it be possible to avoid adding this by using LowHalf == APInt.getWord(0) == APint.trunc(64).getZExtValue() and HighHalf == APInt.getWord(64) == APInt.lshr(64).trunc(64).getZExtValue().
It might help prevent making the internal function public, at the expense of some relatively messier combining back into a __uint128_t.
There was a problem hiding this comment.
This is what the extractBitsAsZExtValue API is for.
There was a problem hiding this comment.
extractBitsAsZExtValue is much better, thanks.
|
I confirmed that this version doesn't have an impact on clang build times. |
davemgreen
approved these changes
Aug 21, 2024
Comment on lines
1793
to
1796
| APInt Api = V.bitcastToAPInt(); | ||
| __uint128_t Int128 = ((__uint128_t)Api.extractBitsAsZExtValue(64, 64) << 64) + | ||
| Api.extractBitsAsZExtValue(64, 0); | ||
| float128 Result = NativeFP(llvm::bit_cast<float128>(Int128)); |
There was a problem hiding this comment.
It might be worth having a method to abstract the two converts in this file.
davemgreen
approved these changes
Aug 22, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/3754 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/3752 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/3590 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/5236 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/185/builds/3838 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/4855 Here is the relevant piece of the build log for the reference: |
MDevereau
added a commit
that referenced
this pull request
Aug 22, 2024
|
ccb2b79 Has been pushed and should tix negative NaN values in tests. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/137/builds/3847 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/3812 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/4239 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/187/builds/587 Here is the relevant piece of the build log for the reference: |
|
@MDevereau I wonder this causes different behavior, for generated binaries by clang, between !HAS_LOGF128 and HAS_LOGF128. My case: I am making my builders check artifacts between "aarch64 object files built by aarch64-clang" and "aarch64 object files built by x86_64-clang for targeting aarch64". This triggers the incompatibility. Investigating. I hope LLVM_HAS_LOGF128 to remain optional, if HAS_LOGF128 would introduce the different behavior. |
|
I would also be ok with a revert of this patch for now. Would that be possible? |
cjdb
pushed a commit
to cjdb/llvm-project
that referenced
this pull request
Aug 23, 2024
…vm#104929) This is a reland of (llvm#96287). This patch attempts to reduce the reverted patch's clang compile time by removing #includes of float128.h and inlining convertToQuad functions instead.
cjdb
pushed a commit
to cjdb/llvm-project
that referenced
this pull request
Aug 23, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/127/builds/617 Here is the relevant piece of the build log for the reference: |
chapuni
reviewed
Aug 25, 2024
| if (!Value.isValidIEEEQuad()) | ||
| return nullptr; | ||
|
|
||
| float128 Result = logf128(ConvertToQuad(Value)); |
There was a problem hiding this comment.
APFloat should have a software emulator that behaves bit-identical.
Then, the native logf128 may be used as an optimization in it.
|
Hello. The behaviour of llvm fp already depends on the host architecture, you have just found another place where that is again true. The compiler has for a long time used the host architectures c library implementation to perform constant folding of fp values. I can appreciate why the determinism you talk about is useful but this is causing some very large performance problems with the difference between fp128 soft-float routines and constant folding. In the long run we would like it to work as you say with an implementation in APFloat, but as far as I understand from looking around that does not exist yet given the current constraints inside llvm (no exp for newton iterations, no sqrt for whatever agm is). The two machines you mention (aarch64 on x86 and aarch64 on aarch64) I would expect should both have logf128 providing they are new enough (or fp128 would not be relevant for them). I believe all the buildbots and reported issues were fixed with the changes from 001e423 and 83a5c7c. It is a shame this got so close before being reverted. |
|
@davemgreen Thanks for your explanation of the background and I am sorry for bothering you.
In fact, my environment, (Ubuntu-20.04 glibc-2.31, gcc-9.4, libstdc++-9), didn't detect x86_64-clang doesn't set For now, I hope logf128-specific behavior may be suppressed with CMake configuration. |
|
It is known that clang on x86-64 doesn't include logf128 via #include cmath (See #96287 (comment)). This was planned to be implemented once this patch landed and will need some extra care to get working. This feature is still a work in progress and is not yet exhaustive for all targets. Thank you for the information about the contents of floatn.h, that's useful to know. ppc is not compatible due to usage of the ibm128 floating point representation rather than IEE754 floats. Thus, we have disabled this for ppc (and the tests now properly do not run now thanks to 001e423). I personally do not have plans to implement a ppc compatible solution for this and will disable it for this host type. |
|
@chapuni Would you be happy with this patch if I relanded it with constant folding enabled by default but with a way to opt out of the behaviour via CMake? |
RoboTux
reviewed
Aug 29, 2024
| check_symbol_exists(__powerpc64le__ "" __PPC64LE) | ||
| if(HAS_LOGF128 AND NOT __PPC64LE) | ||
| set(LLVM_HAS_LOGF128 On) | ||
| add_compile_definitions(HAS_LOGF128) |
There was a problem hiding this comment.
Shouldn't this have a cmakedefine entry in llvm/Config/config.h.cmake instead?
There was a problem hiding this comment.
This was meant to always be enabled if available, and thus cmake would do a test and define the cmake variable LLVM_HAS_LOGF128 if the test passed. This patch has been reverted now, but what is likely to happen is this behaviour will be enabled by default, but be eligible to opt-out via a cmake definition which will have an entry in llvm/Config/config.h.cmake
|
@MDevereau Thanks. I am less happy with it but fair enough since it is configurable. I think we need to discuss. Could you post a survey to the forum before it would be turned on, please? |
MDevereau
added a commit
to MDevereau/llvm-project
that referenced
this pull request
Aug 29, 2024
Hosts with a long double size of 128 bits can benefit highly from constant fp128 folding with the function logf128 This patch relands llvm#104929. With commits 001e423 and 83a5c7c fixing buildbot failures. This patch also adds the option to manually disable the behaviour with the cmake option -DLLVM_DISABLE_LOGF128=On
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/49/builds/391 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/303 Here is the relevant piece of the build log for the reference |
qiaojbao
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Sep 30, 2024
…93b7eeddf Local branch amd-gfx 3b493b7 Merged main:b9a02765504f8b83701ffffc097531638c4fc22e into amd-gfx:3906fefcb801 Remote branch main 3ef64f7 Revert "Enable logf128 constant folding for hosts with 128bit long double (llvm#104929)"
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/35/builds/2031 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/131 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a reland of (#96287). This patch attempts to reduce the reverted patch's clang compile time by removing #includes of float128.h and inlining convertToQuad functions instead.