[APFloat][NFC]extract `fltSemantics::isRepresentableBy` to header #122636

HerrCai0907 · 2025-01-12T13:52:07Z

isRepresentableBy is useful to check float point type compatibility

HerrCai0907 · 2025-01-12T13:52:23Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · 2025-01-12T13:52:55Z

@llvm/pr-subscribers-llvm-support

@llvm/pr-subscribers-llvm-adt

Author: Congcong Cai (HerrCai0907)

Changes

isRepresentableBy is useful to check float point type compatibility

Full diff: https://github.com/llvm/llvm-project/pull/122636.diff

2 Files Affected:

(modified) llvm/include/llvm/ADT/APFloat.h (+5)
(modified) llvm/lib/Support/APFloat.cpp (+9-11)

diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h
index bf80fa5a06580b..528ba391bfc1bc 100644
--- a/llvm/include/llvm/ADT/APFloat.h
+++ b/llvm/include/llvm/ADT/APFloat.h
@@ -281,6 +281,11 @@ struct APFloatBase {
   /// anything real.
   static const fltSemantics &Bogus() LLVM_READNONE;
 
+  // Returns true if any number described by this semantics can be precisely
+  // represented by the specified semantics. Does not take into account
+  // the value of fltNonfiniteBehavior.
+  static bool isRepresentableBy(const fltSemantics &A, const fltSemantics &B);
+
   /// @}
 
   /// IEEE-754R 5.11: Floating Point Comparison Relations.
diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index c9adfca8b3b768..b0d92ae37fe8f6 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -125,14 +125,6 @@ struct fltSemantics {
 
   /* Whether this semantics can represent signed values */
   bool hasSignedRepr = true;
-
-  // Returns true if any number described by this semantics can be precisely
-  // represented by the specified semantics. Does not take into account
-  // the value of fltNonfiniteBehavior.
-  bool isRepresentableBy(const fltSemantics &S) const {
-    return maxExponent <= S.maxExponent && minExponent >= S.minExponent &&
-           precision <= S.precision;
-  }
 };
 
 static constexpr fltSemantics semIEEEhalf = {15, -14, 11, 16};
@@ -290,6 +282,12 @@ const fltSemantics &APFloatBase::x87DoubleExtended() {
 }
 const fltSemantics &APFloatBase::Bogus() { return semBogus; }
 
+bool APFloatBase::isRepresentableBy(const fltSemantics &A,
+                                    const fltSemantics &B) {
+  return A.maxExponent <= B.maxExponent && A.minExponent >= B.minExponent &&
+         A.precision <= B.precision;
+}
+
 constexpr RoundingMode APFloatBase::rmNearestTiesToEven;
 constexpr RoundingMode APFloatBase::rmTowardPositive;
 constexpr RoundingMode APFloatBase::rmTowardNegative;
@@ -5527,7 +5525,7 @@ APFloat::opStatus APFloat::convertToInteger(APSInt &result,
 double APFloat::convertToDouble() const {
   if (&getSemantics() == (const llvm::fltSemantics *)&semIEEEdouble)
     return getIEEE().convertToDouble();
-  assert(getSemantics().isRepresentableBy(semIEEEdouble) &&
+  assert(isRepresentableBy(getSemantics(), semIEEEdouble) &&
          "Float semantics is not representable by IEEEdouble");
   APFloat Temp = *this;
   bool LosesInfo;
@@ -5541,7 +5539,7 @@ double APFloat::convertToDouble() const {
 float128 APFloat::convertToQuad() const {
   if (&getSemantics() == (const llvm::fltSemantics *)&semIEEEquad)
     return getIEEE().convertToQuad();
-  assert(getSemantics().isRepresentableBy(semIEEEquad) &&
+  assert(isRepresentableBy(getSemantics(), semIEEEquad) &&
          "Float semantics is not representable by IEEEquad");
   APFloat Temp = *this;
   bool LosesInfo;
@@ -5555,7 +5553,7 @@ float128 APFloat::convertToQuad() const {
 float APFloat::convertToFloat() const {
   if (&getSemantics() == (const llvm::fltSemantics *)&semIEEEsingle)
     return getIEEE().convertToFloat();
-  assert(getSemantics().isRepresentableBy(semIEEEsingle) &&
+  assert(isRepresentableBy(getSemantics(), semIEEEsingle) &&
          "Float semantics is not representable by IEEEsingle");
   APFloat Temp = *this;
   bool LosesInfo;

kuhar · 2025-01-12T17:52:33Z

@matthias-springer could you confirm if this is compatible with https://discourse.llvm.org/t/rethink-on-approach-to-low-precision-fp-types/82361 . IIUC, it should be.

matthias-springer · 2025-01-12T19:17:29Z

llvm/lib/Support/APFloat.cpp

+bool APFloatBase::isRepresentableBy(const fltSemantics &A,
+                                    const fltSemantics &B) {
+  return A.maxExponent <= B.maxExponent && A.minExponent >= B.minExponent &&
+         A.precision <= B.precision;


I think this should take into account hasZero and hasSignedRepr. Or the documentation should make clear that it doesn't.

matthias-springer · 2025-01-12T19:24:36Z

llvm/include/llvm/ADT/APFloat.h

+  // Returns true if any number described by this semantics can be precisely
+  // represented by the specified semantics. Does not take into account
+  // the value of fltNonfiniteBehavior.
+  static bool isRepresentableBy(const fltSemantics &A, const fltSemantics &B);


What's your use case for this function? Given that this function does not take into account fltNonfiniteBehavior, hasZero, hasSignedRepr, I'm wondering how useful it is in practice. (I'm also wondering, why it does not take into account fltNonfiniteBehavior.)

For the usecase of this functions. I want to warn for implicit loss of accuracy when floating point conversation happened in compiler frontend type systems.
It needs API to detect floating point type compatibility.

(I'm also wondering, why it does not take into account fltNonfiniteBehavior.)

The original usage of this function is in APFloat::convertTo* series function to check compatibility.

At least for these cases, I think it only needs to check the floating point number compatibility in normal range instead to consider NaN and Inf.

Since this patch more like a refactor, I don't want to change the meaning of this API. Actually I try to add these requirements and test break.

As an NFC change, this looks good to me.

If we are changing the semantics of this, can we do that as a separate PR?

matthias-springer · 2025-01-12T19:25:31Z

@matthias-springer could you confirm if this is compatible with https://discourse.llvm.org/t/rethink-on-approach-to-low-precision-fp-types/82361 . IIUC, it should be.

I don't see any problem, either.

durga4github

LGTM for the NFC change

HerrCai0907 · 2025-01-13T22:08:26Z

Merge activity

Jan 13, 5:08 PM EST: A user started a stack merge that includes this pull request via Graphite.
Jan 13, 5:10 PM EST: Graphite rebased this pull request as part of a merge.
Jan 13, 5:12 PM EST: A user merged this pull request with Graphite.

isRepresentableBy is useful to check float point type compatibility

llvm-ci · 2025-01-14T03:26:11Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/11922

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: tools/llvm-gsymutil/ARM_AArch64/macho-merged-funcs-dwarf.yaml' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Input file: /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.dSYM
Output file (aarch64): /b/1/llvm-clang-x86_64-expensive-checks-debian/build/test/tools/llvm-gsymutil/ARM_AArch64/Output/macho-merged-funcs-dwarf.yaml.tmp.default.gSYM
Loaded 3 functions from DWARF.
Loaded 3 functions from symbol table.
warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000047
addr=0x0000000000000248, file=  3, line=  5
addr=0x0000000000000254, file=  3, line=  7
addr=0x0000000000000258, file=  3, line=  9
addr=0x000000000000025c, file=  3, line=  8
addr=0x0000000000000260, file=  3, line= 11
addr=0x0000000000000264, file=  3, line= 10
addr=0x0000000000000268, file=  3, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
addr=0x0000000000000268, file=  2, line=  6


warning: same address range contains different debug info. Removing:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000030
addr=0x0000000000000248, file=  2, line=  5
addr=0x0000000000000254, file=  2, line=  7
addr=0x0000000000000258, file=  2, line=  9
addr=0x000000000000025c, file=  2, line=  8
addr=0x0000000000000260, file=  2, line= 11
addr=0x0000000000000264, file=  2, line= 10
addr=0x0000000000000268, file=  2, line=  6


In favor of this one:
[0x0000000000000248 - 0x0000000000000270): Name=0x00000001
addr=0x0000000000000248, file=  1, line=  5
addr=0x0000000000000254, file=  1, line=  7
addr=0x0000000000000258, file=  1, line=  9
addr=0x000000000000025c, file=  1, line=  8
addr=0x0000000000000260, file=  1, line= 11
addr=0x0000000000000264, file=  1, line= 10
...

HerrCai0907 mentioned this pull request Jan 12, 2025

[clang-tidy] fix wrong float to float conversion check when floating point type is not standard type #122637

Merged

HerrCai0907 marked this pull request as ready for review January 12, 2025 13:52

llvmbot added llvm:support llvm:adt labels Jan 12, 2025

HerrCai0907 requested review from arsenm, chapuni and durga4github January 12, 2025 13:54

kuhar requested a review from matthias-springer January 12, 2025 17:51

matthias-springer reviewed Jan 12, 2025

View reviewed changes

HerrCai0907 force-pushed the users/ccc01-12-_apfloat_extract_fltsemantics_isrepresentableby_to_header branch 2 times, most recently from 43cbcd2 to ded43fc Compare January 13, 2025 00:07

HerrCai0907 changed the title ~~[APFloat]extract fltSemantics::isRepresentableBy to header~~ [APFloat][NFC]extract fltSemantics::isRepresentableBy to header Jan 13, 2025

durga4github approved these changes Jan 13, 2025

View reviewed changes

[APFloat]extract fltSemantics::isRepresentableBy to header

1d0bb33

isRepresentableBy is useful to check float point type compatibility

HerrCai0907 force-pushed the users/ccc01-12-_apfloat_extract_fltsemantics_isrepresentableby_to_header branch from ded43fc to 1d0bb33 Compare January 13, 2025 22:09

HerrCai0907 merged commit ad56f62 into main Jan 13, 2025
4 of 6 checks passed

HerrCai0907 deleted the users/ccc01-12-_apfloat_extract_fltsemantics_isrepresentableby_to_header branch January 13, 2025 22:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[APFloat][NFC]extract `fltSemantics::isRepresentableBy` to header #122636

[APFloat][NFC]extract `fltSemantics::isRepresentableBy` to header #122636

Uh oh!

HerrCai0907 commented Jan 12, 2025

Uh oh!

HerrCai0907 commented Jan 12, 2025

Uh oh!

llvmbot commented Jan 12, 2025 •

edited

Loading

Uh oh!

kuhar commented Jan 12, 2025

Uh oh!

matthias-springer Jan 12, 2025

Uh oh!

matthias-springer Jan 12, 2025

Uh oh!

HerrCai0907 Jan 12, 2025 •

edited

Loading

Uh oh!

durga4github Jan 13, 2025

Uh oh!

durga4github Jan 13, 2025

Uh oh!

matthias-springer commented Jan 12, 2025

Uh oh!

durga4github left a comment

Uh oh!

HerrCai0907 commented Jan 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

llvm-ci commented Jan 14, 2025

Uh oh!

Uh oh!

[APFloat][NFC]extract fltSemantics::isRepresentableBy to header #122636

[APFloat][NFC]extract fltSemantics::isRepresentableBy to header #122636

Uh oh!

Conversation

HerrCai0907 commented Jan 12, 2025

Uh oh!

HerrCai0907 commented Jan 12, 2025

Uh oh!

llvmbot commented Jan 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kuhar commented Jan 12, 2025

Uh oh!

matthias-springer Jan 12, 2025

Choose a reason for hiding this comment

Uh oh!

matthias-springer Jan 12, 2025

Choose a reason for hiding this comment

Uh oh!

HerrCai0907 Jan 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durga4github Jan 13, 2025

Choose a reason for hiding this comment

Uh oh!

durga4github Jan 13, 2025

Choose a reason for hiding this comment

Uh oh!

matthias-springer commented Jan 12, 2025

Uh oh!

durga4github left a comment

Choose a reason for hiding this comment

Uh oh!

HerrCai0907 commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

llvm-ci commented Jan 14, 2025

Uh oh!

Uh oh!

[APFloat][NFC]extract `fltSemantics::isRepresentableBy` to header #122636

[APFloat][NFC]extract `fltSemantics::isRepresentableBy` to header #122636

llvmbot commented Jan 12, 2025 •

edited

Loading

HerrCai0907 Jan 12, 2025 •

edited

Loading

HerrCai0907 commented Jan 13, 2025 •

edited

Loading