Skip to content
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.

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

[C11] Claim conformance to WG14 N1396 #101214

Merged
merged 5 commits into from
Aug 2, 2024

Conversation

AaronBallman
Copy link
Collaborator

The crux of this paper is that floating point expressions can be evaluated in a wider format, but the return statement in a function should still return a value of the function's return type rather than the wider format type.

Note, this is an Annex F conformance requirement and we do not currently claim conformance to Annex F, so technically we conform either way.

The crux of this paper is that floating point expressions can be
evaluated in a wider format, but the return statement in a function
should still return a value of the function's return type rather than
the wider format type.

Note, this is an Annex F conformance requirement and we do not
currently claim conformance to Annex F, so technically we conform
either way.
@AaronBallman AaronBallman added c11 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 30, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jul 30, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 30, 2024

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

The crux of this paper is that floating point expressions can be evaluated in a wider format, but the return statement in a function should still return a value of the function's return type rather than the wider format type.

Note, this is an Annex F conformance requirement and we do not currently claim conformance to Annex F, so technically we conform either way.


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

2 Files Affected:

  • (added) clang/test/C/C11/n1396.c (+140)
  • (modified) clang/www/c_status.html (+1-1)
diff --git a/clang/test/C/C11/n1396.c b/clang/test/C/C11/n1396.c
new file mode 100644
index 0000000000000..311b343e2b715
--- /dev/null
+++ b/clang/test/C/C11/n1396.c
@@ -0,0 +1,140 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+/* WG14 N1396: Clang 15
+ * Wide function returns (alternate proposal)
+ *
+ * This only applies if attempting to conform to Annex F. Clang is not claiming
+ * conformance to Annex F, but we do aim for conformance. This means that the
+ * return statement converts the value to the return type of the function
+ * rather than return the result in a wider evaluation format. We test this by
+ * using a return statement without a cast and ensure it produces the same IR
+ * as a return statement with an explicit cast.
+ *
+ * We claim Clang 15 for conformance because that's the first version that
+ * supported #pragma clang fp eval_method source|extended
+ */
+
+// CHECK-LABEL: define dso_local float @extended_float_func(
+// CHECK-SAME: float noundef [[X:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[X_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:    store float [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[CONV:%.*]] = fpext float [[TMP0]] to double
+// CHECK-NEXT:    [[MUL:%.*]] = fmul double [[CONV]], 1.000000e+00
+// CHECK-NEXT:    [[CONV1:%.*]] = fptrunc double [[MUL]] to float
+// CHECK-NEXT:    ret float [[CONV1]]
+//
+float extended_float_func(float x) {
+#pragma clang fp eval_method(extended)
+  return x * 1.0f;
+}
+
+// CHECK-LABEL: define dso_local float @extended_float_func_cast(
+// CHECK-SAME: float noundef [[X:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[X_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:    store float [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[CONV:%.*]] = fpext float [[TMP0]] to double
+// CHECK-NEXT:    [[MUL:%.*]] = fmul double [[CONV]], 1.000000e+00
+// CHECK-NEXT:    [[CONV1:%.*]] = fptrunc double [[MUL]] to float
+// CHECK-NEXT:    ret float [[CONV1]]
+//
+float extended_float_func_cast(float x) {
+#pragma clang fp eval_method(extended)
+  return (float)(x * 1.0f);
+}
+
+// CHECK-LABEL: define dso_local float @extended_double_func(
+// CHECK-SAME: float noundef [[X:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[X_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:    store float [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[CONV:%.*]] = fpext float [[TMP0]] to double
+// CHECK-NEXT:    [[MUL:%.*]] = fmul double [[CONV]], 1.000000e+00
+// CHECK-NEXT:    [[CONV1:%.*]] = fptrunc double [[MUL]] to float
+// CHECK-NEXT:    ret float [[CONV1]]
+//
+float extended_double_func(float x) {
+#pragma clang fp eval_method(extended)
+  return x * 1.0;
+}
+
+// CHECK-LABEL: define dso_local float @extended_double_func_cast(
+// CHECK-SAME: float noundef [[X:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[X_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:    store float [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[CONV:%.*]] = fpext float [[TMP0]] to double
+// CHECK-NEXT:    [[MUL:%.*]] = fmul double [[CONV]], 1.000000e+00
+// CHECK-NEXT:    [[CONV1:%.*]] = fptrunc double [[MUL]] to float
+// CHECK-NEXT:    ret float [[CONV1]]
+//
+float extended_double_func_cast(float x) {
+#pragma clang fp eval_method(extended)
+  return (float)(x * 1.0);
+}
+
+// CHECK-LABEL: define dso_local float @float_source_func(
+// CHECK-SAME: float noundef [[X:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[X_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:    store float [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[MUL:%.*]] = fmul float [[TMP0]], 1.000000e+00
+// CHECK-NEXT:    ret float [[MUL]]
+//
+float float_source_func(float x) {
+#pragma clang fp eval_method(source)
+  return x * 1.0f;
+}
+
+// CHECK-LABEL: define dso_local float @float_source_func_cast(
+// CHECK-SAME: float noundef [[X:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[X_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:    store float [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[MUL:%.*]] = fmul float [[TMP0]], 1.000000e+00
+// CHECK-NEXT:    ret float [[MUL]]
+//
+float float_source_func_cast(float x) {
+#pragma clang fp eval_method(source)
+  return (float)(x * 1.0f);
+}
+
+// CHECK-LABEL: define dso_local float @double_source_func(
+// CHECK-SAME: float noundef [[X:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[X_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:    store float [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[CONV:%.*]] = fpext float [[TMP0]] to double
+// CHECK-NEXT:    [[MUL:%.*]] = fmul double [[CONV]], 1.000000e+00
+// CHECK-NEXT:    [[CONV1:%.*]] = fptrunc double [[MUL]] to float
+// CHECK-NEXT:    ret float [[CONV1]]
+//
+float double_source_func(float x) {
+#pragma clang fp eval_method(source)
+  return x * 1.0;
+}
+
+// CHECK-LABEL: define dso_local float @double_source_func_cast(
+// CHECK-SAME: float noundef [[X:%.*]]) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[X_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:    store float [[X]], ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[X_ADDR]], align 4
+// CHECK-NEXT:    [[CONV:%.*]] = fpext float [[TMP0]] to double
+// CHECK-NEXT:    [[MUL:%.*]] = fmul double [[CONV]], 1.000000e+00
+// CHECK-NEXT:    [[CONV1:%.*]] = fptrunc double [[MUL]] to float
+// CHECK-NEXT:    ret float [[CONV1]]
+//
+float double_source_func_cast(float x) {
+#pragma clang fp eval_method(source)
+  return (float)(x * 1.0);
+}
diff --git a/clang/www/c_status.html b/clang/www/c_status.html
index 3ea70b0163c70..025bd08712279 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -501,7 +501,7 @@ <h2 id="c11">C11 implementation status</h2>
     <tr>
       <td>Wide function returns (alternate proposal)</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1396.htm">N1396</a></td>
-      <td class="unknown" align="center">Unknown</td>
+      <td class="full" align="center">Clang 15</td>
     </tr>
     <tr id="alignment">
       <td rowspan="3">Alignment</td>

Copy link
Contributor

@jcranmer-intel jcranmer-intel left a comment

Choose a reason for hiding this comment

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

I don't think the test is testing conformance correctly, but I'm not entirely sure how to test conformance correctly.

What the paper is saying is that, on a system like x87 where all evaluation happens in x86_fp80 instead of source types, you need to have an explicit truncation on returning the value.

When compiling on x87 for a no-SSE system, clang doesn't emit any load/store pair for a return x * 1.0f; instruction that would cause the store to be shrunk, not in the same way that it does if you first assign it to a variable: https://godbolt.org/z/G5jff46vs -- in other words, you can see that we do not conform to the expected behavior on x87 no-SSE today.

But... we don't claim compliance to Annex F anyways, so technically we're vacuously conforming to the paper.

The LLVM IR that is generated is probably "correct", as it's the x86 backend that has the bug here. We've never been explicit about what the behavior of floating-point semantics are in LLVM IR (see #44218 and other friends linked there), but there's a general consensus that a) float in LLVM IR means IEEE-754 binary32 with no FLT_EVAL_METHOD shenanigans, b) the x87 backend doesn't conform to that correctly, and c) we're unlikely to fix it, definitely not anytime soon.

After writing all of that, I'm in the mind to say that clang does not conform to this paper because it's failing to apply it in the one case where it actually matters. Another alternative is to just document that we do not intend to ever conform to Annex F on some systems.

@jyknight
Copy link
Member

We aren't compliant for assignment either with x87 float/double -- e.g. enable -O2 on your example.

I think we need to say that we conform except on 32-bit x86 with SSE2 disabled, and link to an appropriate bug. Maybe we'll fix that backend someday. Or maybe not.

@AaronBallman
Copy link
Collaborator Author

Thank you! I updated the test and status information; let me know if you'd like to see further adjustments.

clang/www/c_status.html Outdated Show resolved Hide resolved
@jcranmer-intel
Copy link
Contributor

I mentioned this offline, but worth repeating here: I feel something needs to be marked as partial/no for the x87 case. Aaron suggested Annex F support be that value rather than this paper, which I guess satisfies that.

@AaronBallman
Copy link
Collaborator Author

I mentioned this offline, but worth repeating here: I feel something needs to be marked as partial/no for the x87 case. Aaron suggested Annex F support be that value rather than this paper, which I guess satisfies that.

Agreed; the way I'd like to proceed is to land this (once folks are happy with it) and set the status here to conforming, then update the entry for "IEC 60559 support" (under C99, which is already marked as Partial) to mention 32-bit x86 as another reason why we only partially conform. Does that sound reasonable?

@AaronBallman AaronBallman merged commit cb58294 into llvm:main Aug 2, 2024
7 checks passed
@AaronBallman
Copy link
Collaborator Author

07aaab8 has the changes for updating the status for overall Annex F conformance

banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
The crux of this paper is that floating point expressions can be
evaluated in a wider format, but the return statement in a function
should still return a value of the function's return type rather than
the wider format type.

Note, this is an Annex F conformance requirement and we do not currently
claim conformance to Annex F, so technically we conform either way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c11 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants