[clang] Enable FPContract with optnone #91061
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously treatment of the attribute `optnone` was modified in llvm#85605 ([clang] Set correct FPOptions if attribute 'optnone' presents). As a side effect FPContract was disabled for optnone. It created unneeded divergence with the behavior of -O0, which enables this optimization. In the discussion llvm#85605 (comment) it was pointed out that FP contraction should be enabled even if all optimizations are turned off, otherwise results of calculations would be different. This change enables FPContract at optnone.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Serge Pavlov (spavloff) ChangesPreviously treatment of the attribute In the discussion Full diff: https://github.com/llvm/llvm-project/pull/91061.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index e2a2aa71b880b3..dbf4595790480a 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -970,7 +970,6 @@ class FPOptionsOverride {
void setDisallowOptimizations() {
setFPPreciseEnabled(true);
- setDisallowFPContract();
}
storage_type getAsOpaqueInt() const {
diff --git a/clang/test/AST/ast-dump-fpfeatures.cpp b/clang/test/AST/ast-dump-fpfeatures.cpp
index 68144e31a93043..27558cdac833b6 100644
--- a/clang/test/AST/ast-dump-fpfeatures.cpp
+++ b/clang/test/AST/ast-dump-fpfeatures.cpp
@@ -198,7 +198,7 @@ float func_19(float x, float y) {
// CHECK-LABEL: FunctionDecl {{.*}} func_19 'float (float, float)'
// CHECK: CompoundStmt {{.*}} MathErrno=1
// CHECK: ReturnStmt
-// CHECK: BinaryOperator {{.*}} 'float' '+' ConstRoundingMode=downward MathErrno=1
+// CHECK: BinaryOperator {{.*}} 'float' '+' FPContractMode=1 ConstRoundingMode=downward MathErrno=1
__attribute__((optnone))
float func_20(float x, float y) try {
@@ -210,7 +210,7 @@ float func_20(float x, float y) try {
// CHECK-LABEL: FunctionDecl {{.*}} func_20 'float (float, float)'
// CHECK: CompoundStmt {{.*}} ConstRoundingMode=downward MathErrno=1
// CHECK: ReturnStmt
-// CHECK: BinaryOperator {{.*}} 'float' '+' ConstRoundingMode=downward MathErrno=1
+// CHECK: BinaryOperator {{.*}} 'float' '+' FPContractMode=1 ConstRoundingMode=downward MathErrno=1
struct C21 {
C21(float x, float y);
@@ -221,15 +221,15 @@ struct C21 {
};
// CHECK-LABEL: CXXMethodDecl {{.*}} a_method 'float (float, float)'
-// CHECK: CompoundStmt {{.*}} ConstRoundingMode=downward MathErrno=1
+// CHECK: CompoundStmt {{.*}} FPContractMode=1 ConstRoundingMode=downward MathErrno=1
// CHECK: ReturnStmt
-// CHECK: BinaryOperator {{.*}} 'float' '*' ConstRoundingMode=downward MathErrno=1
+// CHECK: BinaryOperator {{.*}} 'float' '*' FPContractMode=1 ConstRoundingMode=downward MathErrno=1
__attribute__((optnone)) C21::C21(float x, float y) : member(x + y) {}
// CHECK-LABEL: CXXConstructorDecl {{.*}} C21 'void (float, float)'
// CHECK: CXXCtorInitializer {{.*}} 'member' 'float'
-// CHECK: BinaryOperator {{.*}} 'float' '+' ConstRoundingMode=downward MathErrno=1
+// CHECK: BinaryOperator {{.*}} 'float' '+' FPContractMode=1 ConstRoundingMode=downward MathErrno=1
template <typename T>
__attribute__((optnone)) T func_22(T x, T y) {
@@ -238,13 +238,13 @@ __attribute__((optnone)) T func_22(T x, T y) {
// CHECK-LABEL: FunctionTemplateDecl {{.*}} func_22
// CHECK: FunctionDecl {{.*}} func_22 'T (T, T)'
-// CHECK: CompoundStmt {{.*}} ConstRoundingMode=downward MathErrno=1
+// CHECK: CompoundStmt {{.*}} FPContractMode=1 ConstRoundingMode=downward MathErrno=1
// CHECK: ReturnStmt
-// CHECK: BinaryOperator {{.*}} '+' ConstRoundingMode=downward MathErrno=1
+// CHECK: BinaryOperator {{.*}} '+' FPContractMode=1 ConstRoundingMode=downward MathErrno=1
// CHECK: FunctionDecl {{.*}} func_22 'float (float, float)'
-// CHECK: CompoundStmt {{.*}} ConstRoundingMode=downward MathErrno=1
+// CHECK: CompoundStmt {{.*}} FPContractMode=1 ConstRoundingMode=downward MathErrno=1
// CHECK: ReturnStmt
-// CHECK: BinaryOperator {{.*}} 'float' '+' ConstRoundingMode=downward MathErrno=1
+// CHECK: BinaryOperator {{.*}} 'float' '+' FPContractMode=1 ConstRoundingMode=downward MathErrno=1
float func_23(float x, float y) {
return func_22(x, y);
diff --git a/clang/test/AST/ast-dump-fpfeatures.m b/clang/test/AST/ast-dump-fpfeatures.m
index cf77529a756811..2294c0a14a5e38 100644
--- a/clang/test/AST/ast-dump-fpfeatures.m
+++ b/clang/test/AST/ast-dump-fpfeatures.m
@@ -24,6 +24,6 @@ - (float) sum: (float)x with: (float)y __attribute((optnone)) {
// CHECK-LABEL: ObjCImplementationDecl {{.*}} Adder
// CHECK: ObjCMethodDecl {{.*}} - sum:with: 'float'
-// CHECK: CompoundStmt {{.*}} MathErrno=1
+// CHECK: CompoundStmt {{.*}} FPContractMode=1 MathErrno=1
// CHECK-NEXT: ReturnStmt
-// CHECK-NEXT: BinaryOperator {{.*}} 'float' '+' MathErrno=1
+// CHECK-NEXT: BinaryOperator {{.*}} 'float' '+' FPContractMode=1 MathErrno=1
diff --git a/clang/test/AST/ast-dump-late-parsing.cpp b/clang/test/AST/ast-dump-late-parsing.cpp
index 760664efc5f142..c18ba8e8b8c974 100644
--- a/clang/test/AST/ast-dump-late-parsing.cpp
+++ b/clang/test/AST/ast-dump-late-parsing.cpp
@@ -11,13 +11,13 @@ __attribute__((optnone)) T func_22(T x, T y) {
// CHECK-LABEL: FunctionTemplateDecl {{.*}} func_22
// CHECK: FunctionDecl {{.*}} func_22 'T (T, T)'
-// CHECK: CompoundStmt {{.*}} ConstRoundingMode=downward MathErrno=1
+// CHECK: CompoundStmt {{.*}} FPContractMode=1 ConstRoundingMode=downward MathErrno=1
// CHECK: ReturnStmt
-// CHECK: BinaryOperator {{.*}} '+' ConstRoundingMode=downward MathErrno=1
+// CHECK: BinaryOperator {{.*}} '+' FPContractMode=1 ConstRoundingMode=downward MathErrno=1
// CHECK: FunctionDecl {{.*}} func_22 'float (float, float)'
-// CHECK: CompoundStmt {{.*}} ConstRoundingMode=downward MathErrno=1
+// CHECK: CompoundStmt {{.*}} FPContractMode=1 ConstRoundingMode=downward MathErrno=1
// CHECK: ReturnStmt
-// CHECK: BinaryOperator {{.*}} 'float' '+' ConstRoundingMode=downward MathErrno=1
+// CHECK: BinaryOperator {{.*}} 'float' '+' FPContractMode=1 ConstRoundingMode=downward MathErrno=1
float func_23(float x, float y) {
return func_22(x, y);
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
wjristow
reviewed
May 4, 2024
| setFPPreciseEnabled(true); | ||
| setDisallowFPContract(); | ||
| } | ||
| void setDisallowOptimizations() { setFPPreciseEnabled(true); } |
There was a problem hiding this comment.
Actually, instead of just removing the call to setDisallowFPContract(), we should instead explicitly set the FP Contraction state to "on" (which is the default at -O0). That is, we should call setAllowFPContractWithinStatement().
In particular, if the user is compiling with -ffast-math, then the FP Contraction state will be -ffp-contract=fast (allowing FMA across multiple statements). And if they applied optnone, then by removing the call to setDisallowFPContract(), we would mistakenly leave that inter-statement FMA enabled. By calling setAllowFPContractWithinStatement() we are explicitly setting it the equivalent of the command-line option -ffp-contract=on, which is the desired state of -O0 (and optnone).
There was a problem hiding this comment.
Actually this is done by the call to setFPPreciseEnabled: https://github.com/spavloff/llvm-project/blob/e518bbfed9eefbe6d9d952dd0354ee974f7a9305/clang/include/clang/Basic/LangOptions.h#L955-L969
There was a problem hiding this comment.
Ahhh.. I didn't realize. Thanks!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously treatment of the attribute
optnonewas modified in #85605 ([clang] Set correct FPOptions if attribute 'optnone' presents). As a side effect FPContract was disabled for optnone. It created unneeded divergence with the behavior of -O0, which enables this optimization.In the discussion
#85605 (comment) it was pointed out that FP contraction should be enabled even if all optimizations are turned off, otherwise results of calculations would be different. This change enables FPContract at optnone.