[Mlir-commits] [mlir] 68f9d8d - [MLIR][Parser] Fix precision loss when parsing f80 and f128 literals (#220267)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Sep 10 23:50:45 PDT 2026
Author: Chibuoyim (Wilson) Ogbonna
Date: 2026-09-11T08:50:39+02:00
New Revision: 68f9d8d45cad616e223ffe460404edd86dd972ca
URL: https://github.com/llvm/llvm-project/commit/68f9d8d45cad616e223ffe460404edd86dd972ca
DIFF: https://github.com/llvm/llvm-project/commit/68f9d8d45cad616e223ffe460404edd86dd972ca.diff
LOG: [MLIR][Parser] Fix precision loss when parsing f80 and f128 literals (#220267)
While analyzing this CIR issue
[216422](https://github.com/llvm/llvm-project/issues/216422), I traced
the issue to the MLIR asm parser.
For the following case:
```
%0 = arith.constant 1.0E+309 : f80
```
using `mlir-opt repro.mlir`
we get (infinity):
```
module {
%cst = arith.constant 0x7FFF8000000000000000 : f80
}
```
instead of
```
module {
%cst = arith.constant 9.99999999999999999986E+308 : f80
}
```
Currently the parser casts floats to a double before converting to the
requested semantics, as noted in the
[comments](https://github.com/llvm/llvm-project/blob/bb3b509138f4a34cb3d5c659955ccc16745ae82e/mlir/lib/AsmParser/AsmParserImpl.h#L286).
Not entirely sure why this was the default, anyway this patch instead
parses the float with the given semantics directly, so we don't lose
precision.
I used Claude to generate tests and reproducers for this patch. I've
also updated a few existing lit tests to show the improved precision
this patch introduces.
**Note:** This fixes the CIR issue I mentioned above.
Added:
Modified:
mlir/lib/AsmParser/AsmParserImpl.h
mlir/lib/AsmParser/AttributeParser.cpp
mlir/lib/AsmParser/Parser.cpp
mlir/lib/AsmParser/Token.cpp
mlir/lib/AsmParser/Token.h
mlir/test/Dialect/Builtin/Bytecode/attrs.mlir
mlir/test/Dialect/Builtin/Bytecode/builtin_fixed.mlir
mlir/test/Dialect/Builtin/Bytecode/builtin_fixed_0.mlirbc
mlir/test/IR/custom-float-attr-roundtrip.mlir
mlir/test/IR/parser.mlir
Removed:
################################################################################
diff --git a/mlir/lib/AsmParser/AsmParserImpl.h b/mlir/lib/AsmParser/AsmParserImpl.h
index 3c60287a09a0f..e796b3ed2ac13 100644
--- a/mlir/lib/AsmParser/AsmParserImpl.h
+++ b/mlir/lib/AsmParser/AsmParserImpl.h
@@ -283,10 +283,9 @@ class AsmParserImpl : public BaseT {
return success();
}
- /// Parse a floating point value with given semantics from the stream. Since
- /// this implementation parses the string as double precision and only
- /// afterwards converts the value to the requested semantic, precision may be
- /// lost.
+ /// Parse a floating point value with given semantics from the stream. The
+ /// literal is parsed directly using the requested semantics, avoiding range
+ /// or precision loss from an intermediate double conversion.
ParseResult parseFloat(const llvm::fltSemantics &semantics,
APFloat &result) override {
bool isNegative = parser.consumeIf(Token::minus);
diff --git a/mlir/lib/AsmParser/AttributeParser.cpp b/mlir/lib/AsmParser/AttributeParser.cpp
index ba314fac86a37..7459d8e0c3625 100644
--- a/mlir/lib/AsmParser/AttributeParser.cpp
+++ b/mlir/lib/AsmParser/AttributeParser.cpp
@@ -339,10 +339,9 @@ ParseResult Parser::parseAttributeDict(NamedAttrList &attributes) {
/// Parse a float attribute.
Attribute Parser::parseFloatAttr(Type type, bool isNegative) {
- SMLoc loc = getToken().getLoc();
- auto val = getToken().getFloatingPointValue();
- if (!val)
- return (emitError("floating point value too large for attribute"), nullptr);
+ // Defer parsing the literal until the type, and with it the float semantics,
+ // is known.
+ Token literalTok = getToken();
consumeToken(Token::floatliteral);
if (!type) {
// Default to F64 when no type is specified.
@@ -354,15 +353,13 @@ Attribute Parser::parseFloatAttr(Type type, bool isNegative) {
if (!isa<FloatType>(type))
return (emitError("floating point value not valid for specified type"),
nullptr);
- // A type with no signed representation, such as f8E8M0FNU, has no encoding
- // for a negative value. The conversion inside FloatAttr::get keeps the sign
- // bit, and printing the attribute that comes out of it asserts.
- if (isNegative && !APFloat::semanticsHasSignedRepr(
- cast<FloatType>(type).getFloatSemantics()))
- return (emitError(loc, "negative floating point literal for a type with no "
- "signed representation"),
- nullptr);
- return FloatAttr::get(type, isNegative ? -*val : *val);
+ // Note: parseFloatFromLiteral rejects a negative literal for a type with no
+ // signed representation, such as f8E8M0FNU.
+ std::optional<APFloat> result;
+ if (failed(parseFloatFromLiteral(result, literalTok, isNegative,
+ cast<FloatType>(type).getFloatSemantics())))
+ return nullptr;
+ return FloatAttr::get(type, *result);
}
/// Construct an APint from a parsed value, a known attribute type and
diff --git a/mlir/lib/AsmParser/Parser.cpp b/mlir/lib/AsmParser/Parser.cpp
index 22f01c7fdaf82..f1523a154f762 100644
--- a/mlir/lib/AsmParser/Parser.cpp
+++ b/mlir/lib/AsmParser/Parser.cpp
@@ -47,6 +47,7 @@
#include "llvm/Support/Alignment.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Endian.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/PrettyStackTrace.h"
@@ -401,21 +402,25 @@ ParseResult Parser::parseFloatFromLiteral(std::optional<APFloat> &result,
const llvm::fltSemantics &semantics) {
// Check for a floating point value.
if (tok.is(Token::floatliteral)) {
- auto val = tok.getFloatingPointValue();
- if (!val)
- return emitError(tok.getLoc()) << "floating point value too large";
-
// A type with no signed representation, such as f8E8M0FNU, has no encoding
- // for this value at all; the conversion below would keep the sign bit and
+ // for this value at all; negating below would keep the sign bit and
// produce a value that asserts when it is printed.
if (isNegative && !APFloat::semanticsHasSignedRepr(semantics))
return emitError(tok.getLoc())
<< "negative floating point literal for a type with no signed "
"representation";
- result.emplace(isNegative ? -*val : *val);
- bool unused;
- result->convert(semantics, APFloat::rmNearestTiesToEven, &unused);
+ // Parse with the requested semantics directly. Going through a double
+ // first would lose range and precision for wider semantics, such as f80.
+ // The lexer only forms a float literal token for a spelling that APFloat
+ // can parse, so this cannot fail.
+ APFloat value(semantics);
+ llvm::cantFail(value.convertFromString(tok.getSpelling(),
+ APFloat::rmNearestTiesToEven));
+
+ if (isNegative)
+ value.changeSign();
+ result.emplace(std::move(value));
return success();
}
diff --git a/mlir/lib/AsmParser/Token.cpp b/mlir/lib/AsmParser/Token.cpp
index 5bf25bac6a5c6..12f1dca31c416 100644
--- a/mlir/lib/AsmParser/Token.cpp
+++ b/mlir/lib/AsmParser/Token.cpp
@@ -51,15 +51,6 @@ std::optional<uint64_t> Token::getUInt64IntegerValue(StringRef spelling) {
return result;
}
-/// For a floatliteral, return its value as a double. Return std::nullopt if the
-/// value underflows or overflows.
-std::optional<double> Token::getFloatingPointValue() const {
- double result = 0;
- if (spelling.getAsDouble(result))
- return std::nullopt;
- return result;
-}
-
/// For an inttype token, return its bitwidth.
std::optional<unsigned> Token::getIntTypeBitwidth() const {
assert(getKind() == inttype);
diff --git a/mlir/lib/AsmParser/Token.h b/mlir/lib/AsmParser/Token.h
index 9cae80fd51da0..bd8052e4daa87 100644
--- a/mlir/lib/AsmParser/Token.h
+++ b/mlir/lib/AsmParser/Token.h
@@ -84,10 +84,6 @@ class Token {
return getUInt64IntegerValue(getSpelling());
}
- /// For a floatliteral token, return its value as a double. Returns
- /// std::nullopt in the case of underflow or overflow.
- std::optional<double> getFloatingPointValue() const;
-
/// For an inttype token, return its bitwidth.
std::optional<unsigned> getIntTypeBitwidth() const;
diff --git a/mlir/test/Dialect/Builtin/Bytecode/attrs.mlir b/mlir/test/Dialect/Builtin/Bytecode/attrs.mlir
index 7dedfbdff10a7..ee59abc14f1cd 100644
--- a/mlir/test/Dialect/Builtin/Bytecode/attrs.mlir
+++ b/mlir/test/Dialect/Builtin/Bytecode/attrs.mlir
@@ -97,8 +97,8 @@ module @TestDenseStringElementsAttr attributes {
// CHECK-LABEL: @TestFloat
module @TestFloat attributes {
// CHECK: bytecode.float = 1.000000e+01 : f64
- // CHECK: bytecode.float1 = 0.10000{{.*}} : f80
- // CHECK: bytecode.float2 = 0.10000{{.*}} : f128
+ // CHECK: bytecode.float1 = 1.000000e-01 : f80
+ // CHECK: bytecode.float2 = 1.000000e-01 : f128
// CHECK: bytecode.float3 = -5.000000e-01 : bf16
// CHECK: bytecode.inf = 0x7FF0000000000000 : f64
// CHECK: bytecode.nan = 0x7FF8000000000000 : f64
diff --git a/mlir/test/Dialect/Builtin/Bytecode/builtin_fixed.mlir b/mlir/test/Dialect/Builtin/Bytecode/builtin_fixed.mlir
index d427035435277..ebb198456b3c1 100644
--- a/mlir/test/Dialect/Builtin/Bytecode/builtin_fixed.mlir
+++ b/mlir/test/Dialect/Builtin/Bytecode/builtin_fixed.mlir
@@ -125,8 +125,8 @@ module @TestFloatAttr attributes {
// CHECK-DAG: bytecode.f16 = 1.500000e+00 : f16
// CHECK-DAG: bytecode.f32 = 3.140000e+00 : f32
// CHECK-DAG: bytecode.f64 = 1.000000e+01 : f64
- // CHECK-DAG: bytecode.f80 = 0.1{{.*}} : f80
- // CHECK-DAG: bytecode.f128 = 0.1{{.*}} : f128
+ // CHECK-DAG: bytecode.f80 = 1.000000e-01 : f80
+ // CHECK-DAG: bytecode.f128 = 1.000000e-01 : f128
// CHECK-DAG: bytecode.tf32 = 1.000000e+00 : tf32
// CHECK-DAG: bytecode.f8E5M2 = 1.000000e+00 : f8E5M2
// CHECK-DAG: bytecode.f8E4M3 = 1.000000e+00 : f8E4M3
diff --git a/mlir/test/Dialect/Builtin/Bytecode/builtin_fixed_0.mlirbc b/mlir/test/Dialect/Builtin/Bytecode/builtin_fixed_0.mlirbc
index 95be4de8fd1ab..4f661b5a1d233 100644
Binary files a/mlir/test/Dialect/Builtin/Bytecode/builtin_fixed_0.mlirbc and b/mlir/test/Dialect/Builtin/Bytecode/builtin_fixed_0.mlirbc
diff er
diff --git a/mlir/test/IR/custom-float-attr-roundtrip.mlir b/mlir/test/IR/custom-float-attr-roundtrip.mlir
index a8da89ba7372d..ea269e0087be4 100644
--- a/mlir/test/IR/custom-float-attr-roundtrip.mlir
+++ b/mlir/test/IR/custom-float-attr-roundtrip.mlir
@@ -14,6 +14,11 @@ func.func @test_enum_attr_roundtrip() -> () {
"test.op"() {attr = #test.custom_float<"double" : 0x7FF0000001000000>} : () -> ()
// CHECK: attr = #test.custom_float<"fp80" : 0x7FFFC000000000100000>
"test.op"() {attr = #test.custom_float<"fp80" : 0x7FFFC000000000100000>} : () -> ()
+
+ // CHECK: attr = #test.custom_float<"fp80" : 9.99999999999999999986E+308>
+ "test.op"() {attr = #test.custom_float<"fp80" : 1.0E+309>} : () -> ()
+ // CHECK: attr = #test.custom_float<"fp80" : 1.100000e+00>
+ "test.op"() {attr = #test.custom_float<"fp80" : 1.1>} : () -> ()
return
}
diff --git a/mlir/test/IR/parser.mlir b/mlir/test/IR/parser.mlir
index e10dbf36e2797..536f6794c9298 100644
--- a/mlir/test/IR/parser.mlir
+++ b/mlir/test/IR/parser.mlir
@@ -1140,6 +1140,37 @@ func.func @f80_special_values() {
return
}
+// Literals are parsed with the semantics of the target type, so f80 and f128
+// keep the range and precision that would be lost by going through a double.
+// CHECK-LABEL: @wide_float_literals
+func.func @wide_float_literals() {
+ // CHECK: arith.constant 9.99999999999999999986E+308 : f80
+ %0 = arith.constant 1.0E+309 : f80
+ // CHECK: arith.constant -9.99999999999999999986E+308 : f80
+ %1 = arith.constant -1.0E+309 : f80
+ // CHECK: arith.constant 1.000000e+400 : f128
+ %2 = arith.constant 1.0E+400 : f128
+
+ // CHECK: arith.constant 1.100000e+00 : f80
+ %3 = arith.constant 1.1 : f80
+ // CHECK: arith.constant 1.000000e-01 : f128
+ %4 = arith.constant 0.1 : f128
+
+ // CHECK: arith.constant 1.000000e-320 : f80
+ %5 = arith.constant 1.0E-320 : f80
+
+ // Out of range for the target type: still infinity.
+ // CHECK: arith.constant 0x7FFF8000000000000000 : f80
+ %6 = arith.constant 1.0E+5000 : f80
+ // CHECK: arith.constant 0x7FF0000000000000 : f64
+ %7 = arith.constant 1.0E+400 : f64
+
+ // CHECK: arith.constant dense<[1.100000e+00, 9.99999999999999999986E+308]> : tensor<2xf80>
+ %8 = arith.constant dense<[1.1, 1.0E+309]> : tensor<2xf80>
+
+ return
+}
+
// We want to print floats in exponential notation with 6 significant digits,
// but it may lead to precision loss when parsing back, in which case we print
// the decimal form instead.
More information about the Mlir-commits
mailing list