Skip to content

Enable logf128 constant folding for hosts with 128bit long double #104929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 22, 2024
Merged

Enable logf128 constant folding for hosts with 128bit long double #104929

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6bd959c
Enable logf128 constant folding for hosts with 128bit long double
MDevereau Aug 19, 2024
77bae12
Use Api.extractBitsAsZExtValue
MDevereau Aug 20, 2024
b45d146
Remove stray semi-colon
MDevereau Aug 20, 2024
5961bc5
Move common code to ConvertToQuad
MDevereau Aug 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions llvm/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
18 changes: 7 additions & 11 deletions llvm/cmake/config-ix.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Copy link
Contributor

@RoboTux RoboTux Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this have a cmakedefine entry in llvm/Config/config.h.cmake instead?

Copy link
Contributor Author

@MDevereau MDevereau Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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)
Expand Down
15 changes: 3 additions & 12 deletions llvm/include/llvm/ADT/APFloat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down Expand Up @@ -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;

/// @}
Expand Down Expand Up @@ -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.
///
Expand Down
20 changes: 6 additions & 14 deletions llvm/include/llvm/ADT/APInt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Copy link
Collaborator

@davemgreen davemgreen Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@nikic nikic Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what the extractBitsAsZExtValue API is for.

Copy link
Contributor Author

@MDevereau MDevereau Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractBitsAsZExtValue is much better, thanks.

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.
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 6 additions & 8 deletions llvm/include/llvm/Support/float128.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 0 additions & 6 deletions llvm/lib/Analysis/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
24 changes: 19 additions & 5 deletions llvm/lib/Analysis/ConstantFolding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
24 changes: 2 additions & 22 deletions llvm/lib/Support/APFloat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down