llvm / llvm/llvm-project

`KnownFPClass` and `APFloat` disagree on the definition of `subnormal` for `PPCDoubleDouble`.

Open
#220,808 1 comment 0 reactions 0 assignees View on GitHub
floating-point llvm:support
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

The code for `KnownFPClass` is written with two assumptions about `fcSubnormal`:
1. `subnormal` values have a non-zero magnitude strictly less than `1.0`*.
2. `fcNegNormal < fcNegSubnormal < fcZero < fcPosSubnormal < fcPosNormal`.

*Or in practice, a value with a non-zero magnitude less than ~`0.01`, since the largest minimum normal value for a "common" floating point type is `2^-14 == 0.000061` for `ieee_binary16`.

However, `APFloat::classify` uses `APFloat::isDenormal` to determine if a value is `subnormal`. This is problematic for the `PPCDoubleDouble` type as values such as `{+1.0, DBL_TRUE_MIN}` are considered as `fcSubnormal` by `APFloat::classify`, breaking both of the common assumptions relied upon by `KnownFPClass`.

https://github.com/llvm/llvm-project/blob/0f391cb05e6e4727453b5ace37eda77ce67db85f/llvm/lib/Support/APFloat.cpp#L5716-L5721

https://github.com/llvm/llvm-project/blob/0f391cb05e6e4727453b5ace37eda77ce67db85f/llvm/lib/Support/APFloat.cpp#L5939-L5950

For `PPCDoubleDouble`, I was assuming that `fcSubnormal` was classified in one of two ways:
- The value is subnormal if the high part is subnormal.
- The value is subnormal if the value is non-zero and has a magnitude strictly less than `DBL_MIN * 2^53` (where it is unable to represent values that would require 106 bits of precision).

***

Possible solutions:
- Add `APFloat::isSubnormal`
- Add `fcDenormal`/`kfcDenormal` (As a follow up to the sequence of PRs outlined by https://github.com/llvm/llvm-project/issues/217072).
- Pass the floating point semantics into each `KnownFPClass` function, and return `fcAllFlags`/unknown if we have not tested or implemented the function for a given floating point semantics (could also be done via virtual functions or templates). This would allow `KnownFPClass` to only support the types we have properly verified, instead of excluding the types we have found bugs for.

As a follow up, it may be good to consider if non-canonical x87 forms should have dedicated classification flags (such as unnormal numbers, pseudo-subnormal, pseudo-NaN, etc)

Contributor guide

Open the contributing guide

Research direction

Start with the linked sections of llvm/lib/Support/APFloat.cpp, especially APFloat::classify and isDenormal, then trace the KnownFPClass assumptions about fcSubnormal. Determine and document a consistent classification for PPCDoubleDouble, with the chosen solution resolving the disagreement without breaking the stated ordering or magnitude assumptions.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.