[APFloat][NFCI] Move x87DoubleExtended next to the other native IEEE types

[ZERICO2005]

Reordered the APFloatBase::Semantics so that S_x87DoubleExtended is placed after the IEEE types and before the DoubleDouble types.

Previously, S_x87DoubleExtended was the very last enum value, meaning that SemanticsToEnum/bitcastToAPInt/initFromAPInt would test if Sem is one of the 13 GPU floating point types before testing if Sem is x87DoubleExtended. Given that x87DoubleExtended is more commonly used and a native LLVM IR type, I thought a better heuristic would be to test if Sem is a x87DoubleExtended type before testing if it is one of the 13 GPU floating point types.

I also made the order of other functions/fields more consistent across APFloat.

[llvmorg-github-actions]

@llvm/pr-subscribers-llvm-adt

@llvm/pr-subscribers-llvm-support

Author: Eric Ross (ZERICO2005)

Changes

Reordered the APFloatBase::Semantics so that S_x87DoubleExtended is placed after the IEEE types and before the DoubleDouble types.

Previously, S_x87DoubleExtended was the very last enum value, meaning that SemanticsToEnum/bitcastToAPInt/initFromAPInt would test if Sem is one of the 13 GPU floating point types before testing if Sem is x87DoubleExtended. Given that x87DoubleExtended is more commonly used and a native LLVM IR type, I thought a better heuristic would be to test if Sem is a x87DoubleExtended type before testing if it is one of the 13 GPU floating point types.

I also made the order of other functions/fields more consistent across APFloat.

---

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

2 Files Affected:

• (modified) llvm/include/llvm/ADT/APFloat.h (+9-9)
• (modified) llvm/lib/Support/APFloat.cpp (+26-25)

diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h
index b51b6256ad2c6..9b903fc888b4c 100644
--- a/llvm/include/llvm/ADT/APFloat.h
+++ b/llvm/include/llvm/ADT/APFloat.h
@@ -163,6 +163,8 @@ class APFloatBase {
     S_IEEEsingle,
     S_IEEEdouble,
     S_IEEEquad,
+    // TODO: Documentation is missing.
+    S_x87DoubleExtended,
     // The IBM double-double semantics. Such a number consists of a pair of
     // IEEE 64-bit doubles (Hi, Lo), where |Hi| > |Lo|, and if normal,
     // (double)(Hi + Lo) == Hi. The numeric value it's modeling is Hi + Lo.
@@ -262,9 +264,7 @@ class APFloatBase {
     // Unlike IEEE-754 types, there are no infinity values, and NaN is
     // represented with the exponent and mantissa bits set to all 1s.
     S_Float8E5M3FNU,
-    // TODO: Documentation is missing.
-    S_x87DoubleExtended,
-    S_MaxSemantics = S_x87DoubleExtended,
+    S_MaxSemantics = S_Float8E5M3FNU,
   };
 
   LLVM_ABI static const llvm::fltSemantics &EnumToSemantics(Semantics S);
@@ -276,6 +276,9 @@ class APFloatBase {
   LLVM_ABI static const fltSemantics semIEEEsingle;
   LLVM_ABI static const fltSemantics semIEEEdouble;
   LLVM_ABI static const fltSemantics semIEEEquad;
+  LLVM_ABI static const fltSemantics semX87DoubleExtended;
+  LLVM_ABI static const fltSemantics semPPCDoubleDouble;
+  LLVM_ABI static const fltSemantics semPPCDoubleDoubleLegacy;
   LLVM_ABI static const fltSemantics semFloat8E5M2;
   LLVM_ABI static const fltSemantics semFloat8E5M2FNUZ;
   LLVM_ABI static const fltSemantics semFloat8E4M3;
@@ -289,10 +292,7 @@ class APFloatBase {
   LLVM_ABI static const fltSemantics semFloat6E3M2FN;
   LLVM_ABI static const fltSemantics semFloat6E2M3FN;
   LLVM_ABI static const fltSemantics semFloat4E2M1FN;
-  LLVM_ABI static const fltSemantics semX87DoubleExtended;
   LLVM_ABI static const fltSemantics semBogus;
-  LLVM_ABI static const fltSemantics semPPCDoubleDouble;
-  LLVM_ABI static const fltSemantics semPPCDoubleDoubleLegacy;
 
   friend class detail::IEEEFloat;
   friend class detail::DoubleAPFloat;
@@ -304,6 +304,9 @@ class APFloatBase {
   static const fltSemantics &IEEEsingle() { return semIEEEsingle; }
   static const fltSemantics &IEEEdouble() { return semIEEEdouble; }
   static const fltSemantics &IEEEquad() { return semIEEEquad; }
+  static const fltSemantics &x87DoubleExtended() {
+    return semX87DoubleExtended;
+  }
   static const fltSemantics &PPCDoubleDouble() { return semPPCDoubleDouble; }
   static const fltSemantics &PPCDoubleDoubleLegacy() {
     return semPPCDoubleDoubleLegacy;
@@ -323,9 +326,6 @@ class APFloatBase {
   static const fltSemantics &Float6E3M2FN() { return semFloat6E3M2FN; }
   static const fltSemantics &Float6E2M3FN() { return semFloat6E2M3FN; }
   static const fltSemantics &Float4E2M1FN() { return semFloat4E2M1FN; }
-  static const fltSemantics &x87DoubleExtended() {
-    return semX87DoubleExtended;
-  }
 
   /// A Pseudo fltsemantic used to construct APFloats that cannot conflict with
   /// anything real.
diff --git a/llvm/lib/Support/APFloat.cpp b/llvm/lib/Support/APFloat.cpp
index 59a6ba3867d40..45ee43a595e56 100644
--- a/llvm/lib/Support/APFloat.cpp
+++ b/llvm/lib/Support/APFloat.cpp
@@ -73,6 +73,21 @@ constexpr fltSemantics APFloatBase::semBFloat = {127, -126, 8, 16};
 constexpr fltSemantics APFloatBase::semIEEEsingle = {127, -126, 24, 32};
 constexpr fltSemantics APFloatBase::semIEEEdouble = {1023, -1022, 53, 64};
 constexpr fltSemantics APFloatBase::semIEEEquad = {16383, -16382, 113, 128};
+constexpr fltSemantics APFloatBase::semX87DoubleExtended = {
+    16383,
+    -16382,
+    64,
+    80,
+    fltNonfiniteBehavior::IEEE754,
+    fltNanEncoding::IEEE,
+    true,
+    true,
+    true,
+    true,
+    true};
+constexpr fltSemantics APFloatBase::semPPCDoubleDouble = {-1, 0, 0, 128};
+constexpr fltSemantics APFloatBase::semPPCDoubleDoubleLegacy = {
+    1023, -1022 + 53, 53 + 53, 128};
 constexpr fltSemantics APFloatBase::semFloat8E5M2 = {15, -14, 3, 8};
 constexpr fltSemantics APFloatBase::semFloat8E5M2FNUZ = {
     15, -15, 3, 8, fltNonfiniteBehavior::NanOnly, fltNanEncoding::NegativeZero};
@@ -114,22 +129,7 @@ constexpr fltSemantics APFloatBase::semFloat6E2M3FN = {
     2, 0, 4, 6, fltNonfiniteBehavior::FiniteOnly};
 constexpr fltSemantics APFloatBase::semFloat4E2M1FN = {
     2, 0, 2, 4, fltNonfiniteBehavior::FiniteOnly};
-constexpr fltSemantics APFloatBase::semX87DoubleExtended = {
-    16383,
-    -16382,
-    64,
-    80,
-    fltNonfiniteBehavior::IEEE754,
-    fltNanEncoding::IEEE,
-    true,
-    true,
-    true,
-    true,
-    true};
 constexpr fltSemantics APFloatBase::semBogus = {0, 0, 0, 0};
-constexpr fltSemantics APFloatBase::semPPCDoubleDouble = {-1, 0, 0, 128};
-constexpr fltSemantics APFloatBase::semPPCDoubleDoubleLegacy = {
-    1023, -1022 + 53, 53 + 53, 128};
 
 const llvm::fltSemantics &APFloatBase::EnumToSemantics(Semantics S) {
   switch (S) {
@@ -143,6 +143,8 @@ const llvm::fltSemantics &APFloatBase::EnumToSemantics(Semantics S) {
     return IEEEdouble();
   case S_IEEEquad:
     return IEEEquad();
+  case S_x87DoubleExtended:
+    return x87DoubleExtended();
   case S_PPCDoubleDouble:
     return PPCDoubleDouble();
   case S_PPCDoubleDoubleLegacy:
@@ -173,8 +175,6 @@ const llvm::fltSemantics &APFloatBase::EnumToSemantics(Semantics S) {
     return Float6E2M3FN();
   case S_Float4E2M1FN:
     return Float4E2M1FN();
-  case S_x87DoubleExtended:
-    return x87DoubleExtended();
   }
   llvm_unreachable("Unrecognised floating semantics");
 }
@@ -191,6 +191,8 @@ APFloatBase::SemanticsToEnum(const llvm::fltSemantics &Sem) {
     return S_IEEEdouble;
   else if (&Sem == &llvm::APFloat::IEEEquad())
     return S_IEEEquad;
+  else if (&Sem == &llvm::APFloat::x87DoubleExtended())
+    return S_x87DoubleExtended;
   else if (&Sem == &llvm::APFloat::PPCDoubleDouble())
     return S_PPCDoubleDouble;
   else if (&Sem == &llvm::APFloat::PPCDoubleDoubleLegacy())
@@ -221,8 +223,6 @@ APFloatBase::SemanticsToEnum(const llvm::fltSemantics &Sem) {
     return S_Float6E2M3FN;
   else if (&Sem == &llvm::APFloat::Float4E2M1FN())
     return S_Float4E2M1FN;
-  else if (&Sem == &llvm::APFloat::x87DoubleExtended())
-    return S_x87DoubleExtended;
   else
     llvm_unreachable("Unknown floating semantics");
 }
@@ -3691,6 +3691,10 @@ APInt IEEEFloat::bitcastToAPInt() const {
   if (semantics == (const llvm::fltSemantics *)&APFloatBase::semIEEEquad)
     return convertQuadrupleAPFloatToAPInt();
 
+  if (semantics ==
+      (const llvm::fltSemantics *)&APFloatBase::semX87DoubleExtended)
+    return convertF80LongDoubleAPFloatToAPInt();
+
   if (semantics ==
       (const llvm::fltSemantics *)&APFloatBase::semPPCDoubleDoubleLegacy)
     return convertPPCDoubleDoubleLegacyAPFloatToAPInt();
@@ -3735,10 +3739,7 @@ APInt IEEEFloat::bitcastToAPInt() const {
   if (semantics == (const llvm::fltSemantics *)&APFloatBase::semFloat4E2M1FN)
     return convertFloat4E2M1FNAPFloatToAPInt();
 
-  assert(semantics ==
-             (const llvm::fltSemantics *)&APFloatBase::semX87DoubleExtended &&
-         "unknown format!");
-  return convertF80LongDoubleAPFloatToAPInt();
+  llvm_unreachable("unknown format!");
 }
 
 float IEEEFloat::convertToFloat() const {
@@ -4017,10 +4018,10 @@ void IEEEFloat::initFromAPInt(const fltSemantics *Sem, const APInt &api) {
     return initFromFloatAPInt(api);
   if (Sem == &APFloatBase::semIEEEdouble)
     return initFromDoubleAPInt(api);
-  if (Sem == &APFloatBase::semX87DoubleExtended)
-    return initFromF80LongDoubleAPInt(api);
   if (Sem == &APFloatBase::semIEEEquad)
     return initFromQuadrupleAPInt(api);
+  if (Sem == &APFloatBase::semX87DoubleExtended)
+    return initFromF80LongDoubleAPInt(api);
   if (Sem == &APFloatBase::semPPCDoubleDoubleLegacy)
     return initFromPPCDoubleDoubleLegacyAPInt(api);
   if (Sem == &APFloatBase::semFloat8E5M2)

[ZERICO2005]

The order of the GPU floating point types is not very consistent in APFloat. Here semFloat4E2M1FN is the last type, while S_Float8E5M3FNU is the last enum. Might be good to apply a consistent ordering in the future.

[ZERICO2005]

Note: Turned assert into llvm_unreachable here.

[ZERICO2005]

One could also argue that float and double are more commonly used than half and bfloat. So perhaps we could test float/double before half/bfloat in the future.

[ojhunt]

Seems like a reasonable change - x87's fp80 is explicitly an ieee754 format.