Skip to content

[clang] Use constant rounding mode for floating literals #90877

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 10 commits into from
May 17, 2024
Merged

[clang] Use constant rounding mode for floating literals #90877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5d906b5
[clang] Use constant rounding mode for floating literals
spavloff May 2, 2024
a6fad25
Fix typo
spavloff May 3, 2024
f2e0414
Add test for NTTP
spavloff May 3, 2024
cf6ff55
Replace dynamic rounding mode with NearestTiesToEven
spavloff May 3, 2024
aeb6074
Add additional test for float literal as NTTP
spavloff May 3, 2024
ed25d8a
Fix clang-format error
spavloff May 3, 2024
da2795c
Add test for template instatiations
spavloff May 3, 2024
58cbc15
Add test for literal in macro
spavloff May 4, 2024
50173a8
Reorder tests
spavloff May 6, 2024
6ce8363
Add tests for explicit instantiation and specialization
spavloff May 6, 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
10 changes: 4 additions & 6 deletions clang/include/clang/Lex/LiteralSupport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,10 @@ class NumericLiteralParser {
/// bits of the result and return true. Otherwise, return false.
bool GetIntegerValue(llvm::APInt &Val);

/// GetFloatValue - Convert this numeric literal to a floating value, using
/// the specified APFloat fltSemantics (specifying float, double, etc).
/// The optional bool isExact (passed-by-reference) has its value
/// set to true if the returned APFloat can represent the number in the
/// literal exactly, and false otherwise.
llvm::APFloat::opStatus GetFloatValue(llvm::APFloat &Result);
/// Convert this numeric literal to a floating value, using the specified
/// APFloat fltSemantics (specifying float, double, etc) and rounding mode.
llvm::APFloat::opStatus GetFloatValue(llvm::APFloat &Result,
llvm::RoundingMode RM);

/// GetFixedPointValue - Convert this numeric literal value into a
/// scaled integer that represents this value. Returns true if an overflow
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Lex/LiteralSupport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,8 @@ bool NumericLiteralParser::GetIntegerValue(llvm::APInt &Val) {
}

llvm::APFloat::opStatus
NumericLiteralParser::GetFloatValue(llvm::APFloat &Result) {
NumericLiteralParser::GetFloatValue(llvm::APFloat &Result,
llvm::RoundingMode RM) {
using llvm::APFloat;

unsigned n = std::min(SuffixBegin - ThisTokBegin, ThisTokEnd - ThisTokBegin);
Expand All @@ -1534,8 +1535,7 @@ NumericLiteralParser::GetFloatValue(llvm::APFloat &Result) {
Str = Buffer;
}

auto StatusOrErr =
Result.convertFromString(Str, APFloat::rmNearestTiesToEven);
auto StatusOrErr = Result.convertFromString(Str, RM);
assert(StatusOrErr && "Invalid floating point representation");
return !errorToBool(StatusOrErr.takeError()) ? *StatusOrErr
: APFloat::opInvalidOp;
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3858,7 +3858,10 @@ static Expr *BuildFloatingLiteral(Sema &S, NumericLiteralParser &Literal,
using llvm::APFloat;
APFloat Val(Format);

APFloat::opStatus result = Literal.GetFloatValue(Val);
llvm::RoundingMode RM = S.CurFPFeatures.getRoundingMode();
if (RM == llvm::RoundingMode::Dynamic)
RM = llvm::RoundingMode::NearestTiesToEven;
APFloat::opStatus result = Literal.GetFloatValue(Val, RM);

// Overflow is always an error, but underflow is only an error if
// we underflowed to zero (APFloat reports denormals as underflow).
Expand Down
6 changes: 6 additions & 0 deletions clang/test/AST/const-fpfeatures.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ float FI1u = 0xFFFFFFFFU;
float _Complex C1u = C0;
// CHECK: @C1u = {{.*}} { float, float } { float 0x3FF0000020000000, float 0x3FF0000020000000 }

float FLu = 0.1F;
// CHECK: @FLu = {{.*}} float 0x3FB99999A0000000


#pragma STDC FENV_ROUND FE_DOWNWARD

Expand All @@ -35,3 +38,6 @@ float FI1d = 0xFFFFFFFFU;

float _Complex C1d = C0;
// CHECK: @C1d = {{.*}} { float, float } { float 1.000000e+00, float 1.000000e+00 }

float FLd = 0.1F;
// CHECK: @FLd = {{.*}} float 0x3FB9999980000000
105 changes: 105 additions & 0 deletions clang/test/AST/const-fpfeatures.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,108 @@ float V7 = []() -> float {
0x0.000001p0F);
}();
// CHECK: @V7 = {{.*}} float 1.000000e+00

#pragma STDC FENV_ROUND FE_DYNAMIC

template<float V> struct L {
constexpr L() : value(V) {}
float value;
};

#pragma STDC FENV_ROUND FE_DOWNWARD
Copy link
Collaborator

@erichkeane erichkeane May 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I meant something like:

template<typename T, T V> void foo() {
#pragma STDC FENV_ROUND FE_DOWNWARD
T Val = V;
// Should be:  { float 0x3FB9999980000000 }
}
#pragma STDC FENV_ROUND FE_UPDWARD
foo<float, 0.1F>();

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see that github ate the first line of my comment, see above, i'v eedited it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this snippet Val in foo<float, 0.1F>() should be float 0x3FB99999A0000000 (rounded upward), because the literal 0.1F is in the region, where #pragma STDC FENV_ROUND FE_UPDWARD is in effect. The body of foo does not contain literals.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this snippet Val in foo<float, 0.1F>() should be float 0x3FB99999A0000000 (rounded upward), because the literal 0.1F is in the region, where #pragma STDC FENV_ROUND FE_UPDWARD is in effect. The body of foo does not contain literals.

Hmm... ok, then the feature isn't quite what I expected it was. Does the FENV_ROUND exclusively change the behavior of literals? I thought it messed with math as well? (I thought it was on assignment, but perhaps not!).

I'm more concerned about a case where the point-of-instantiation and the template have different rounding modes. But I see this patch is exclusively for literals, so we can let that go.

One thing to ensure: can we have an AST test (like your 'foo' example) that shows that they are different instantiations? Are there any cases where we could get two instantiation-requests spelled differently, but because of different rounding modes, are the same instantiation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does the FENV_ROUND exclusively change the behavior of literals? I thought it messed with math as well?

Of course, FENV_ROUND affects any math, but this patch fixes misbehavior of literals only. And the note was about different expectation.

Assignment isn't an operation that depends on rounding mode, but changing the snippet to such:

template<typename T, T V> void foo() {
#pragma STDC FENV_ROUND FE_DOWNWARD
T Val = V + 1;
}

would make use of the rounding mode. In this case the addition in foo would be evaluated using rounding down, but the literal is converted to AST object using rounding up.

I'm more concerned about a case where the point-of-instantiation and the template have different rounding modes.

Does this test address your concern: https://github.com/spavloff/llvm-project/blob/aeb6074444513587924106081213335f73ba6eb0/clang/test/AST/ast-dump-fpfeatures.cpp#L124-L143 ?

One thing to ensure: can we have an AST test (like your 'foo' example) that shows that they are different instantiations? Are there any cases where we could get two instantiation-requests spelled differently, but because of different rounding modes, are the same instantiation?

All instantiations of a function template use the rounding mode specified in the template definition. Otherwise we would have bad things like two instantiations in different translation units that use different rounding modes. The rounding mode is attached to AST nodes in the AST representation for function templates, tests in AST/ast-dump-fpfeatures.cpp check this property.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this test address your concern: https://github.com/spavloff/llvm-project/blob/aeb6074444513587924106081213335f73ba6eb0/clang/test/AST/ast-dump-fpfeatures.cpp#L124-L143 ?

That one does not. I'm more looking for:

template<typename T, T f> void foo(){}
#pragma STDC FENV_ROUND FE_DOWNWARD
foo<float, 0.1f>();
#pragma STDC FENV_ROUND FE_UPWARD
foo<float, 0.1f>();

Then confirming that there are two separate instantiations of foo.

All instantiations of a function template use the rounding mode specified in the template definition. Otherwise we would have bad things like two instantiations in different translation units that use different rounding modes.

Right, that was very much my concern, the ODR implications of that are scary. So jsut the 1 test above, and I think I am good about this, though having @jcranmer-intel and/or @zahiraam review would also be appreciated.

Copy link
Contributor

Choose a reason for hiding this comment

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

The interaction of these pragmas with C++ is underspecified, although for the most part, you can make pretty reasonable expectations about what the results should be. I'm also planning on bringing a paper to C++ to clarify the semantics somewhat, with the intent mostly being "just don't allow the pragma in most places."

That said, templates do add in all sorts of fun extra complexity, and there are at least three scenarios worth testing (with regards to interpretation of constants within the template body):

  • Implicit template instantiation (you already test this)
  • Explicit template instantiation
  • Template specialization

I can definitely agree that implicit template instantiation should inherit from the rounding mode of the definition. Template specialization probably shouldn't inherit from the original template definition. Explicit instantiation... I'm not sure? I can argue that one either way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Explicit instantiation also should use the rounding mode at the point of definition. It is possible that the template is instantiated in two translation units and rounding mode is different at the points of instantiation.

L<0.1F> val_d;
// CHECK: @val_d = {{.*}} { float 0x3FB9999980000000 }

#pragma STDC FENV_ROUND FE_UPWARD
L<0.1F> val_u;
// CHECK: @val_u = {{.*}} { float 0x3FB99999A0000000 }


// Check literals in macros.

#pragma STDC FENV_ROUND FE_DOWNWARD
#define CONSTANT_0_1 0.1F

#pragma STDC FENV_ROUND FE_UPWARD
float C1_ru = CONSTANT_0_1;
// CHECK: @C1_ru = {{.*}} float 0x3FB99999A0000000

#pragma STDC FENV_ROUND FE_DOWNWARD
float C1_rd = CONSTANT_0_1;
// CHECK: @C1_rd = {{.*}} float 0x3FB9999980000000

#pragma STDC FENV_ROUND FE_DOWNWARD
#define PRAGMA(x) _Pragma(#x)
#define CONSTANT_0_1_RM(v, rm) ([](){ PRAGMA(STDC FENV_ROUND rm); return v; }())

#pragma STDC FENV_ROUND FE_UPWARD
float C2_rd = CONSTANT_0_1_RM(0.1F, FE_DOWNWARD);
float C2_ru = CONSTANT_0_1_RM(0.1F, FE_UPWARD);
// CHECK: @C2_rd = {{.*}} float 0x3FB9999980000000
// CHECK: @C2_ru = {{.*}} float 0x3FB99999A0000000

#pragma STDC FENV_ROUND FE_DOWNWARD
float C3_rd = CONSTANT_0_1_RM(0.1F, FE_DOWNWARD);
float C3_ru = CONSTANT_0_1_RM(0.1F, FE_UPWARD);
// CHECK: @C3_rd = {{.*}} float 0x3FB9999980000000
// CHECK: @C3_ru = {{.*}} float 0x3FB99999A0000000

// Check literals in template instantiations.

#pragma STDC FENV_ROUND FE_DYNAMIC

template<typename T, T C>
constexpr T foo() {
return C;
}

#pragma STDC FENV_ROUND FE_DOWNWARD
float var_d = foo<float, 0.1F>();
// CHECK: @var_d = {{.*}} float 0x3FB9999980000000

#pragma STDC FENV_ROUND FE_UPWARD
float var_u = foo<float, 0.1F>();
// CHECK: @var_u = {{.*}} float 0x3FB99999A0000000

#pragma STDC FENV_ROUND FE_DYNAMIC

template<typename T, T f> void foo2() {
T Val = f;
}

void func_01() {
#pragma STDC FENV_ROUND FE_DOWNWARD
foo2<float, 0.1f>();
}

void func_02() {
#pragma STDC FENV_ROUND FE_UPWARD
foo2<float, 0.1f>();
}

// CHECK-LABEL: define {{.*}} void @_Z4foo2IfTnT_Lf3dccccccEEvv()
// CHECK: store float 0x3FB9999980000000, ptr

// CHECK-LABEL: define {{.*}} void @_Z4foo2IfTnT_Lf3dcccccdEEvv()
// CHECK: store float 0x3FB99999A0000000, ptr


#pragma STDC FENV_ROUND FE_DOWNWARD
template <int C>
float tfunc_01() {
return 0.1F; // Must be 0x3FB9999980000000 in all instantiations.
}
template float tfunc_01<0>();
// CHECK-LABEL: define {{.*}} float @_Z8tfunc_01ILi0EEfv()
// CHECK: ret float 0x3FB9999980000000

#pragma STDC FENV_ROUND FE_UPWARD
template float tfunc_01<1>();
// CHECK-LABEL: define {{.*}} float @_Z8tfunc_01ILi1EEfv()
// CHECK: ret float 0x3FB9999980000000

template<> float tfunc_01<2>() {
return 0.1F;
}
// CHECK-LABEL: define {{.*}} float @_Z8tfunc_01ILi2EEfv()
// CHECK: ret float 0x3FB99999A0000000