-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Flang] Add -ffast-real-mod back for further control of MOD optimizations #167118
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
base: main
Are you sure you want to change the base?
Conversation
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Michael Klemm (mjklemm) ChangesIt turns out that having Full diff: https://github.com/llvm/llvm-project/pull/167118.diff 8 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 11e81e032d5fc..fcefc236b9d82 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2778,6 +2778,9 @@ def fno_unsafe_math_optimizations : Flag<["-"], "fno-unsafe-math-optimizations">
Group<f_Group>;
def fassociative_math : Flag<["-"], "fassociative-math">, Visibility<[ClangOption, FlangOption]>, Group<f_Group>;
def fno_associative_math : Flag<["-"], "fno-associative-math">, Visibility<[ClangOption, FlangOption]>, Group<f_Group>;
+def ffast_real_mod : Flag<["-"], "ffast-real-mod">,
+ Group<f_Group>, Visibility<[FlangOption, FC1Option]>,
+ HelpText<"Enable optimization of MOD for REAL types">;
def fno_fast_real_mod : Flag<["-"], "fno-fast-real-mod">,
Group<f_Group>, Visibility<[FlangOption, FC1Option]>,
HelpText<"Disable optimization of MOD for REAL types in presence of -ffast-math">;
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 88bce181d40d2..c5230e5502cda 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -822,6 +822,8 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
complexRangeKindToStr(Range)));
}
+ if (Args.hasArg(options::OPT_ffast_real_mod))
+ CmdArgs.push_back("-ffast-real-mod");
if (Args.hasArg(options::OPT_fno_fast_real_mod))
CmdArgs.push_back("-fno-fast-real-mod");
diff --git a/flang/include/flang/Support/LangOptions.def b/flang/include/flang/Support/LangOptions.def
index e7185c836f45b..e310ecf37a52d 100644
--- a/flang/include/flang/Support/LangOptions.def
+++ b/flang/include/flang/Support/LangOptions.def
@@ -61,7 +61,7 @@ LANGOPT(OpenMPNoNestedParallelism, 1, 0)
/// Use SIMD only OpenMP support.
LANGOPT(OpenMPSimd, 1, false)
/// Enable fast MOD operations for REAL
-LANGOPT(NoFastRealMod, 1, false)
+LANGOPT(FastRealMod, 1, false)
LANGOPT(VScaleMin, 32, 0) ///< Minimum vscale range value
LANGOPT(VScaleMax, 32, 0) ///< Maximum vscale range value
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index f05c4cfccf7fc..bea3dfc6638a8 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1428,11 +1428,14 @@ static bool parseFloatingPointArgs(CompilerInvocation &invoc,
opts.ReciprocalMath = true;
opts.ApproxFunc = true;
opts.NoSignedZeros = true;
+ opts.FastRealMod = true;
opts.setFPContractMode(Fortran::common::LangOptions::FPM_Fast);
}
+ if (args.hasArg(clang::driver::options::OPT_ffast_real_mod))
+ opts.FastRealMod = true;
if (args.hasArg(clang::driver::options::OPT_fno_fast_real_mod))
- opts.NoFastRealMod = true;
+ opts.FastRealMod = false;
return true;
}
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 159d08a2797b3..ddf125f9bb216 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -277,11 +277,11 @@ bool CodeGenAction::beginSourceFileAction() {
ci.getInvocation().getLangOpts().OpenMPVersion);
}
- if (ci.getInvocation().getLangOpts().NoFastRealMod) {
+ if (ci.getInvocation().getLangOpts().FastRealMod) {
mlir::ModuleOp mod = lb.getModule();
mod.getOperation()->setAttr(
mlir::StringAttr::get(mod.getContext(),
- llvm::Twine{"fir.no_fast_real_mod"}),
+ llvm::Twine{"fir.fast_real_mod"}),
mlir::BoolAttr::get(mod.getContext(), true));
}
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 3eb60448fae38..9c8dbd6a65b4d 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -6509,11 +6509,9 @@ static mlir::Value genFastMod(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value IntrinsicLibrary::genMod(mlir::Type resultType,
llvm::ArrayRef<mlir::Value> args) {
auto mod = builder.getModule();
- bool dontUseFastRealMod = false;
- bool canUseApprox = mlir::arith::bitEnumContainsAny(
- builder.getFastMathFlags(), mlir::arith::FastMathFlags::afn);
- if (auto attr = mod->getAttrOfType<mlir::BoolAttr>("fir.no_fast_real_mod"))
- dontUseFastRealMod = attr.getValue();
+ bool useFastRealMod = false;
+ if (auto attr = mod->getAttrOfType<mlir::BoolAttr>("fir.fast_real_mod"))
+ useFastRealMod = attr.getValue();
assert(args.size() == 2);
if (resultType.isUnsignedInteger()) {
@@ -6526,7 +6524,7 @@ mlir::Value IntrinsicLibrary::genMod(mlir::Type resultType,
if (mlir::isa<mlir::IntegerType>(resultType))
return mlir::arith::RemSIOp::create(builder, loc, args[0], args[1]);
- if (resultType.isFloat() && canUseApprox && !dontUseFastRealMod) {
+ if (resultType.isFloat() && useFastRealMod) {
// Treat MOD as an approximate function and code-gen inline code
// instead of calling into the Fortran runtime library.
return builder.createConvert(loc, resultType,
diff --git a/flang/test/Driver/fast-real-mod.f90 b/flang/test/Driver/fast-real-mod.f90
index 4ea9b26e64753..8184f334c3d85 100644
--- a/flang/test/Driver/fast-real-mod.f90
+++ b/flang/test/Driver/fast-real-mod.f90
@@ -1,5 +1,7 @@
+! RUN: %flang -ffast-real-mod -### -c %s 2>&1 | FileCheck %s -check-prefix CHECK-FAST-REAL-MOD
! RUN: %flang -fno-fast-real-mod -### -c %s 2>&1 | FileCheck %s -check-prefix CHECK-NO-FAST-REAL-MOD
+! CHECK-FAST-REAL-MOD: "-ffast-real-mod"
! CHECK-NO-FAST-REAL-MOD: "-fno-fast-real-mod"
program test
diff --git a/flang/test/Lower/Intrinsics/fast-real-mod.f90 b/flang/test/Lower/Intrinsics/fast-real-mod.f90
index f80f7203ad1a2..8613c0eb3c2f6 100644
--- a/flang/test/Lower/Intrinsics/fast-real-mod.f90
+++ b/flang/test/Lower/Intrinsics/fast-real-mod.f90
@@ -1,24 +1,24 @@
-! RUN: %flang_fc1 -ffast-math -emit-mlir -o - %s | FileCheck %s --check-prefixes=CHECK%if target=x86_64{{.*}} %{,CHECK-KIND10%}%if flang-supports-f128-math %{,CHECK-KIND16%}
+! RUN: %flang_fc1 -ffast-real-mod -emit-mlir -o - %s | FileCheck %s --check-prefixes=CHECK-FRM%if target=x86_64{{.*}} %{,CHECK-FRM-KIND10%}%if flang-supports-f128-math %{,CHECK-FRM-KIND16%}
+! RUN: %flang_fc1 -ffast-math -emit-mlir -o - %s | FileCheck %s --check-prefixes=CHECK-FRM%if target=x86_64{{.*}} %{,CHECK-FRM-KIND10%}%if flang-supports-f128-math %{,CHECK-FRM-KIND16%}
! RUN: %flang_fc1 -ffast-math -fno-fast-real-mod -emit-mlir -o - %s | FileCheck %s --check-prefixes=CHECK-NFRM%if target=x86_64{{.*}} %{,CHECK-NFRM-KIND10%}%if flang-supports-f128-math %{,CHECK-NFRM-KIND16%}
-! TODO: check line that fir.fast_real_mod is not there
-! CHECK-NFRM: module attributes {{{.*}}fir.no_fast_real_mod = true{{.*}}}
+! CHECK,CHECK-FRM: module attributes {{{.*}}fir.fast_real_mod = true{{.*}}}
! CHECK-LABEL: @_QPmod_real4
subroutine mod_real4(r, a, p)
implicit none
real(kind=4) :: r, a, p
-! CHECK: %[[A:.*]] = fir.declare{{.*}}a"
-! CHECK: %[[P:.*]] = fir.declare{{.*}}p"
-! CHECK: %[[R:.*]] = fir.declare{{.*}}r"
-! CHECK: %[[A_LOAD:.*]] = fir.load %[[A]]
-! CHECK: %[[P_LOAD:.*]] = fir.load %[[P]]
-! CHECK: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<fast> : f32
-! CHECK: %[[CV1:.*]] = fir.convert %[[DIV]] : (f32) -> si32
-! CHECK: %[[CV2:.*]] = fir.convert %[[CV1]] : (si32) -> f32
-! CHECK: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<fast> : f32
-! CHECK: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<fast> : f32
-! CHECK: fir.store %[[SUB]] to %[[R]] : !fir.ref<f32>
+! CHECK-FRM: %[[A:.*]] = fir.declare{{.*}}a"
+! CHECK-FRM: %[[P:.*]] = fir.declare{{.*}}p"
+! CHECK-FRM: %[[R:.*]] = fir.declare{{.*}}r"
+! CHECK-FRM: %[[A_LOAD:.*]] = fir.load %[[A]]
+! CHECK-FRM: %[[P_LOAD:.*]] = fir.load %[[P]]
+! CHECK-FRM: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f32
+! CHECK-FRM: %[[CV1:.*]] = fir.convert %[[DIV]] : (f32) -> si32
+! CHECK-FRM: %[[CV2:.*]] = fir.convert %[[CV1]] : (si32) -> f32
+! CHECK-FRM: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<{{.*}}> : f32
+! CHECK-FRM: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<{{.*}}> : f32
+! CHECK-FRM: fir.store %[[SUB]] to %[[R]] : !fir.ref<f32>
! CHECK-NFRM: fir.call @_FortranAModReal4(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}}: (f32, f32, !fir.ref<i8>, i32) -> f32
r = mod(a, p)
end subroutine mod_real4
@@ -27,17 +27,17 @@ end subroutine mod_real4
subroutine mod_real8(r, a, p)
implicit none
real(kind=8) :: r, a, p
-! CHECK: %[[A:.*]] = fir.declare{{.*}}a"
-! CHECK: %[[P:.*]] = fir.declare{{.*}}p"
-! CHECK: %[[R:.*]] = fir.declare{{.*}}r"
-! CHECK: %[[A_LOAD:.*]] = fir.load %[[A]]
-! CHECK: %[[P_LOAD:.*]] = fir.load %[[P]]
-! CHECK: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<fast> : f64
-! CHECK: %[[CV1:.*]] = fir.convert %[[DIV]] : (f64) -> si64
-! CHECK: %[[CV2:.*]] = fir.convert %[[CV1]] : (si64) -> f64
-! CHECK: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<fast> : f64
-! CHECK: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<fast> : f64
-! CHECK: fir.store %[[SUB]] to %[[R]] : !fir.ref<f64>
+! CHECK-FRM: %[[A:.*]] = fir.declare{{.*}}a"
+! CHECK-FRM: %[[P:.*]] = fir.declare{{.*}}p"
+! CHECK-FRM: %[[R:.*]] = fir.declare{{.*}}r"
+! CHECK-FRM: %[[A_LOAD:.*]] = fir.load %[[A]]
+! CHECK-FRM: %[[P_LOAD:.*]] = fir.load %[[P]]
+! CHECK-FRM: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f64
+! CHECK-FRM: %[[CV1:.*]] = fir.convert %[[DIV]] : (f64) -> si64
+! CHECK-FRM: %[[CV2:.*]] = fir.convert %[[CV1]] : (si64) -> f64
+! CHECK-FRM: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<{{.*}}> : f64
+! CHECK-FRM: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<{{.*}}> : f64
+! CHECK-FRM: fir.store %[[SUB]] to %[[R]] : !fir.ref<f64>
! CHECK-NFRM: fir.call @_FortranAModReal8(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}}: (f64, f64, !fir.ref<i8>, i32) -> f64
r = mod(a, p)
end subroutine mod_real8
@@ -47,17 +47,17 @@ subroutine mod_real10(r, a, p)
implicit none
integer, parameter :: kind10 = merge(10, 4, selected_real_kind(p=18).eq.10)
real(kind=kind10) :: r, a, p
-! CHECK-KIND10: %[[A:.*]] = fir.declare{{.*}}a"
-! CHECK-KIND10: %[[P:.*]] = fir.declare{{.*}}p"
-! CHECK-KIND10: %[[R:.*]] = fir.declare{{.*}}r"
-! CHECK-KIND10: %[[A_LOAD:.*]] = fir.load %[[A]]
-! CHECK-KIND10: %[[P_LOAD:.*]] = fir.load %[[P]]
-! CHECK-KIND10: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<fast> : f80
-! CHECK-KIND10: %[[CV1:.*]] = fir.convert %[[DIV]] : (f80) -> si80
-! CHECK-KIND10: %[[CV2:.*]] = fir.convert %[[CV1]] : (si80) -> f80
-! CHECK-KIND10: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<fast> : f80
-! CHECK-KIND10: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<fast> : f80
-! CHECK-KIND10: fir.store %[[SUB]] to %[[R]] : !fir.ref<f80>
+! CHECK-FRM-KIND10: %[[A:.*]] = fir.declare{{.*}}a"
+! CHECK-FRM-KIND10: %[[P:.*]] = fir.declare{{.*}}p"
+! CHECK-FRM-KIND10: %[[R:.*]] = fir.declare{{.*}}r"
+! CHECK-FRM-KIND10: %[[A_LOAD:.*]] = fir.load %[[A]]
+! CHECK-FRM-KIND10: %[[P_LOAD:.*]] = fir.load %[[P]]
+! CHECK-FRM-KIND10: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f80
+! CHECK-FRM-KIND10: %[[CV1:.*]] = fir.convert %[[DIV]] : (f80) -> si80
+! CHECK-FRM-KIND10: %[[CV2:.*]] = fir.convert %[[CV1]] : (si80) -> f80
+! CHECK-FRM-KIND10: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<{{.*}}> : f80
+! CHECK-FRM-KIND10: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<{{.*}}> : f80
+! CHECK-FRM-KIND10: fir.store %[[SUB]] to %[[R]] : !fir.ref<f80>
! CHECK-NFRM-KIND10: fir.call @_FortranAModReal10(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}}: (f80, f80, !fir.ref<i8>, i32) -> f80
r = mod(a, p)
end subroutine mod_real10
@@ -67,17 +67,17 @@ subroutine mod_real16(r, a, p)
implicit none
integer, parameter :: kind16 = merge(16, 4, selected_real_kind(p=33).eq.16)
real(kind=kind16) :: r, a, p
-! CHECK-KIND16: %[[A:.*]] = fir.declare{{.*}}a"
-! CHECK-KIND16: %[[P:.*]] = fir.declare{{.*}}p"
-! CHECK-KIND16: %[[R:.*]] = fir.declare{{.*}}r"
-! CHECK-KIND16: %[[A_LOAD:.*]] = fir.load %[[A]]
-! CHECK-KIND16: %[[P_LOAD:.*]] = fir.load %[[P]]
-! CHECK-KIND16: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<fast> : f128
-! CHECK-KIND16: %[[CV1:.*]] = fir.convert %[[DIV]] : (f128) -> si128
-! CHECK-KIND16: %[[CV2:.*]] = fir.convert %[[CV1]] : (si128) -> f128
-! CHECK-KIND16: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<fast> : f128
-! CHECK-KIND16: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<fast> : f128
-! CHECK-KIND16: fir.store %[[SUB]] to %[[R]] : !fir.ref<f128>
+! CHECK-FRM-KIND16: %[[A:.*]] = fir.declare{{.*}}a"
+! CHECK-FRM-KIND16: %[[P:.*]] = fir.declare{{.*}}p"
+! CHECK-FRM-KIND16: %[[R:.*]] = fir.declare{{.*}}r"
+! CHECK-FRM-KIND16: %[[A_LOAD:.*]] = fir.load %[[A]]
+! CHECK-FRM-KIND16: %[[P_LOAD:.*]] = fir.load %[[P]]
+! CHECK-FRM-KIND16: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f128
+! CHECK-FRM-KIND16: %[[CV1:.*]] = fir.convert %[[DIV]] : (f128) -> si128
+! CHECK-FRM-KIND16: %[[CV2:.*]] = fir.convert %[[CV1]] : (si128) -> f128
+! CHECK-FRM-KIND16: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<{{.*}}> : f128
+! CHECK-FRM-KIND16: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<{{.*}}> : f128
+! CHECK-FRM-KIND16: fir.store %[[SUB]] to %[[R]] : !fir.ref<f128>
! CHECK-NFRM-KIND16: fir.call @_FortranAModReal16(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}}: (f128, f128, !fir.ref<i8>, i32) -> f128
r = mod(a, p)
end subroutine mod_real16
|
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Michael Klemm (mjklemm) ChangesIt turns out that having Full diff: https://github.com/llvm/llvm-project/pull/167118.diff 8 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 11e81e032d5fc..fcefc236b9d82 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2778,6 +2778,9 @@ def fno_unsafe_math_optimizations : Flag<["-"], "fno-unsafe-math-optimizations">
Group<f_Group>;
def fassociative_math : Flag<["-"], "fassociative-math">, Visibility<[ClangOption, FlangOption]>, Group<f_Group>;
def fno_associative_math : Flag<["-"], "fno-associative-math">, Visibility<[ClangOption, FlangOption]>, Group<f_Group>;
+def ffast_real_mod : Flag<["-"], "ffast-real-mod">,
+ Group<f_Group>, Visibility<[FlangOption, FC1Option]>,
+ HelpText<"Enable optimization of MOD for REAL types">;
def fno_fast_real_mod : Flag<["-"], "fno-fast-real-mod">,
Group<f_Group>, Visibility<[FlangOption, FC1Option]>,
HelpText<"Disable optimization of MOD for REAL types in presence of -ffast-math">;
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 88bce181d40d2..c5230e5502cda 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -822,6 +822,8 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
complexRangeKindToStr(Range)));
}
+ if (Args.hasArg(options::OPT_ffast_real_mod))
+ CmdArgs.push_back("-ffast-real-mod");
if (Args.hasArg(options::OPT_fno_fast_real_mod))
CmdArgs.push_back("-fno-fast-real-mod");
diff --git a/flang/include/flang/Support/LangOptions.def b/flang/include/flang/Support/LangOptions.def
index e7185c836f45b..e310ecf37a52d 100644
--- a/flang/include/flang/Support/LangOptions.def
+++ b/flang/include/flang/Support/LangOptions.def
@@ -61,7 +61,7 @@ LANGOPT(OpenMPNoNestedParallelism, 1, 0)
/// Use SIMD only OpenMP support.
LANGOPT(OpenMPSimd, 1, false)
/// Enable fast MOD operations for REAL
-LANGOPT(NoFastRealMod, 1, false)
+LANGOPT(FastRealMod, 1, false)
LANGOPT(VScaleMin, 32, 0) ///< Minimum vscale range value
LANGOPT(VScaleMax, 32, 0) ///< Maximum vscale range value
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index f05c4cfccf7fc..bea3dfc6638a8 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1428,11 +1428,14 @@ static bool parseFloatingPointArgs(CompilerInvocation &invoc,
opts.ReciprocalMath = true;
opts.ApproxFunc = true;
opts.NoSignedZeros = true;
+ opts.FastRealMod = true;
opts.setFPContractMode(Fortran::common::LangOptions::FPM_Fast);
}
+ if (args.hasArg(clang::driver::options::OPT_ffast_real_mod))
+ opts.FastRealMod = true;
if (args.hasArg(clang::driver::options::OPT_fno_fast_real_mod))
- opts.NoFastRealMod = true;
+ opts.FastRealMod = false;
return true;
}
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 159d08a2797b3..ddf125f9bb216 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -277,11 +277,11 @@ bool CodeGenAction::beginSourceFileAction() {
ci.getInvocation().getLangOpts().OpenMPVersion);
}
- if (ci.getInvocation().getLangOpts().NoFastRealMod) {
+ if (ci.getInvocation().getLangOpts().FastRealMod) {
mlir::ModuleOp mod = lb.getModule();
mod.getOperation()->setAttr(
mlir::StringAttr::get(mod.getContext(),
- llvm::Twine{"fir.no_fast_real_mod"}),
+ llvm::Twine{"fir.fast_real_mod"}),
mlir::BoolAttr::get(mod.getContext(), true));
}
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 3eb60448fae38..9c8dbd6a65b4d 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -6509,11 +6509,9 @@ static mlir::Value genFastMod(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::Value IntrinsicLibrary::genMod(mlir::Type resultType,
llvm::ArrayRef<mlir::Value> args) {
auto mod = builder.getModule();
- bool dontUseFastRealMod = false;
- bool canUseApprox = mlir::arith::bitEnumContainsAny(
- builder.getFastMathFlags(), mlir::arith::FastMathFlags::afn);
- if (auto attr = mod->getAttrOfType<mlir::BoolAttr>("fir.no_fast_real_mod"))
- dontUseFastRealMod = attr.getValue();
+ bool useFastRealMod = false;
+ if (auto attr = mod->getAttrOfType<mlir::BoolAttr>("fir.fast_real_mod"))
+ useFastRealMod = attr.getValue();
assert(args.size() == 2);
if (resultType.isUnsignedInteger()) {
@@ -6526,7 +6524,7 @@ mlir::Value IntrinsicLibrary::genMod(mlir::Type resultType,
if (mlir::isa<mlir::IntegerType>(resultType))
return mlir::arith::RemSIOp::create(builder, loc, args[0], args[1]);
- if (resultType.isFloat() && canUseApprox && !dontUseFastRealMod) {
+ if (resultType.isFloat() && useFastRealMod) {
// Treat MOD as an approximate function and code-gen inline code
// instead of calling into the Fortran runtime library.
return builder.createConvert(loc, resultType,
diff --git a/flang/test/Driver/fast-real-mod.f90 b/flang/test/Driver/fast-real-mod.f90
index 4ea9b26e64753..8184f334c3d85 100644
--- a/flang/test/Driver/fast-real-mod.f90
+++ b/flang/test/Driver/fast-real-mod.f90
@@ -1,5 +1,7 @@
+! RUN: %flang -ffast-real-mod -### -c %s 2>&1 | FileCheck %s -check-prefix CHECK-FAST-REAL-MOD
! RUN: %flang -fno-fast-real-mod -### -c %s 2>&1 | FileCheck %s -check-prefix CHECK-NO-FAST-REAL-MOD
+! CHECK-FAST-REAL-MOD: "-ffast-real-mod"
! CHECK-NO-FAST-REAL-MOD: "-fno-fast-real-mod"
program test
diff --git a/flang/test/Lower/Intrinsics/fast-real-mod.f90 b/flang/test/Lower/Intrinsics/fast-real-mod.f90
index f80f7203ad1a2..8613c0eb3c2f6 100644
--- a/flang/test/Lower/Intrinsics/fast-real-mod.f90
+++ b/flang/test/Lower/Intrinsics/fast-real-mod.f90
@@ -1,24 +1,24 @@
-! RUN: %flang_fc1 -ffast-math -emit-mlir -o - %s | FileCheck %s --check-prefixes=CHECK%if target=x86_64{{.*}} %{,CHECK-KIND10%}%if flang-supports-f128-math %{,CHECK-KIND16%}
+! RUN: %flang_fc1 -ffast-real-mod -emit-mlir -o - %s | FileCheck %s --check-prefixes=CHECK-FRM%if target=x86_64{{.*}} %{,CHECK-FRM-KIND10%}%if flang-supports-f128-math %{,CHECK-FRM-KIND16%}
+! RUN: %flang_fc1 -ffast-math -emit-mlir -o - %s | FileCheck %s --check-prefixes=CHECK-FRM%if target=x86_64{{.*}} %{,CHECK-FRM-KIND10%}%if flang-supports-f128-math %{,CHECK-FRM-KIND16%}
! RUN: %flang_fc1 -ffast-math -fno-fast-real-mod -emit-mlir -o - %s | FileCheck %s --check-prefixes=CHECK-NFRM%if target=x86_64{{.*}} %{,CHECK-NFRM-KIND10%}%if flang-supports-f128-math %{,CHECK-NFRM-KIND16%}
-! TODO: check line that fir.fast_real_mod is not there
-! CHECK-NFRM: module attributes {{{.*}}fir.no_fast_real_mod = true{{.*}}}
+! CHECK,CHECK-FRM: module attributes {{{.*}}fir.fast_real_mod = true{{.*}}}
! CHECK-LABEL: @_QPmod_real4
subroutine mod_real4(r, a, p)
implicit none
real(kind=4) :: r, a, p
-! CHECK: %[[A:.*]] = fir.declare{{.*}}a"
-! CHECK: %[[P:.*]] = fir.declare{{.*}}p"
-! CHECK: %[[R:.*]] = fir.declare{{.*}}r"
-! CHECK: %[[A_LOAD:.*]] = fir.load %[[A]]
-! CHECK: %[[P_LOAD:.*]] = fir.load %[[P]]
-! CHECK: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<fast> : f32
-! CHECK: %[[CV1:.*]] = fir.convert %[[DIV]] : (f32) -> si32
-! CHECK: %[[CV2:.*]] = fir.convert %[[CV1]] : (si32) -> f32
-! CHECK: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<fast> : f32
-! CHECK: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<fast> : f32
-! CHECK: fir.store %[[SUB]] to %[[R]] : !fir.ref<f32>
+! CHECK-FRM: %[[A:.*]] = fir.declare{{.*}}a"
+! CHECK-FRM: %[[P:.*]] = fir.declare{{.*}}p"
+! CHECK-FRM: %[[R:.*]] = fir.declare{{.*}}r"
+! CHECK-FRM: %[[A_LOAD:.*]] = fir.load %[[A]]
+! CHECK-FRM: %[[P_LOAD:.*]] = fir.load %[[P]]
+! CHECK-FRM: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f32
+! CHECK-FRM: %[[CV1:.*]] = fir.convert %[[DIV]] : (f32) -> si32
+! CHECK-FRM: %[[CV2:.*]] = fir.convert %[[CV1]] : (si32) -> f32
+! CHECK-FRM: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<{{.*}}> : f32
+! CHECK-FRM: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<{{.*}}> : f32
+! CHECK-FRM: fir.store %[[SUB]] to %[[R]] : !fir.ref<f32>
! CHECK-NFRM: fir.call @_FortranAModReal4(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}}: (f32, f32, !fir.ref<i8>, i32) -> f32
r = mod(a, p)
end subroutine mod_real4
@@ -27,17 +27,17 @@ end subroutine mod_real4
subroutine mod_real8(r, a, p)
implicit none
real(kind=8) :: r, a, p
-! CHECK: %[[A:.*]] = fir.declare{{.*}}a"
-! CHECK: %[[P:.*]] = fir.declare{{.*}}p"
-! CHECK: %[[R:.*]] = fir.declare{{.*}}r"
-! CHECK: %[[A_LOAD:.*]] = fir.load %[[A]]
-! CHECK: %[[P_LOAD:.*]] = fir.load %[[P]]
-! CHECK: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<fast> : f64
-! CHECK: %[[CV1:.*]] = fir.convert %[[DIV]] : (f64) -> si64
-! CHECK: %[[CV2:.*]] = fir.convert %[[CV1]] : (si64) -> f64
-! CHECK: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<fast> : f64
-! CHECK: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<fast> : f64
-! CHECK: fir.store %[[SUB]] to %[[R]] : !fir.ref<f64>
+! CHECK-FRM: %[[A:.*]] = fir.declare{{.*}}a"
+! CHECK-FRM: %[[P:.*]] = fir.declare{{.*}}p"
+! CHECK-FRM: %[[R:.*]] = fir.declare{{.*}}r"
+! CHECK-FRM: %[[A_LOAD:.*]] = fir.load %[[A]]
+! CHECK-FRM: %[[P_LOAD:.*]] = fir.load %[[P]]
+! CHECK-FRM: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f64
+! CHECK-FRM: %[[CV1:.*]] = fir.convert %[[DIV]] : (f64) -> si64
+! CHECK-FRM: %[[CV2:.*]] = fir.convert %[[CV1]] : (si64) -> f64
+! CHECK-FRM: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<{{.*}}> : f64
+! CHECK-FRM: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<{{.*}}> : f64
+! CHECK-FRM: fir.store %[[SUB]] to %[[R]] : !fir.ref<f64>
! CHECK-NFRM: fir.call @_FortranAModReal8(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}}: (f64, f64, !fir.ref<i8>, i32) -> f64
r = mod(a, p)
end subroutine mod_real8
@@ -47,17 +47,17 @@ subroutine mod_real10(r, a, p)
implicit none
integer, parameter :: kind10 = merge(10, 4, selected_real_kind(p=18).eq.10)
real(kind=kind10) :: r, a, p
-! CHECK-KIND10: %[[A:.*]] = fir.declare{{.*}}a"
-! CHECK-KIND10: %[[P:.*]] = fir.declare{{.*}}p"
-! CHECK-KIND10: %[[R:.*]] = fir.declare{{.*}}r"
-! CHECK-KIND10: %[[A_LOAD:.*]] = fir.load %[[A]]
-! CHECK-KIND10: %[[P_LOAD:.*]] = fir.load %[[P]]
-! CHECK-KIND10: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<fast> : f80
-! CHECK-KIND10: %[[CV1:.*]] = fir.convert %[[DIV]] : (f80) -> si80
-! CHECK-KIND10: %[[CV2:.*]] = fir.convert %[[CV1]] : (si80) -> f80
-! CHECK-KIND10: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<fast> : f80
-! CHECK-KIND10: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<fast> : f80
-! CHECK-KIND10: fir.store %[[SUB]] to %[[R]] : !fir.ref<f80>
+! CHECK-FRM-KIND10: %[[A:.*]] = fir.declare{{.*}}a"
+! CHECK-FRM-KIND10: %[[P:.*]] = fir.declare{{.*}}p"
+! CHECK-FRM-KIND10: %[[R:.*]] = fir.declare{{.*}}r"
+! CHECK-FRM-KIND10: %[[A_LOAD:.*]] = fir.load %[[A]]
+! CHECK-FRM-KIND10: %[[P_LOAD:.*]] = fir.load %[[P]]
+! CHECK-FRM-KIND10: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f80
+! CHECK-FRM-KIND10: %[[CV1:.*]] = fir.convert %[[DIV]] : (f80) -> si80
+! CHECK-FRM-KIND10: %[[CV2:.*]] = fir.convert %[[CV1]] : (si80) -> f80
+! CHECK-FRM-KIND10: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<{{.*}}> : f80
+! CHECK-FRM-KIND10: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<{{.*}}> : f80
+! CHECK-FRM-KIND10: fir.store %[[SUB]] to %[[R]] : !fir.ref<f80>
! CHECK-NFRM-KIND10: fir.call @_FortranAModReal10(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}}: (f80, f80, !fir.ref<i8>, i32) -> f80
r = mod(a, p)
end subroutine mod_real10
@@ -67,17 +67,17 @@ subroutine mod_real16(r, a, p)
implicit none
integer, parameter :: kind16 = merge(16, 4, selected_real_kind(p=33).eq.16)
real(kind=kind16) :: r, a, p
-! CHECK-KIND16: %[[A:.*]] = fir.declare{{.*}}a"
-! CHECK-KIND16: %[[P:.*]] = fir.declare{{.*}}p"
-! CHECK-KIND16: %[[R:.*]] = fir.declare{{.*}}r"
-! CHECK-KIND16: %[[A_LOAD:.*]] = fir.load %[[A]]
-! CHECK-KIND16: %[[P_LOAD:.*]] = fir.load %[[P]]
-! CHECK-KIND16: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<fast> : f128
-! CHECK-KIND16: %[[CV1:.*]] = fir.convert %[[DIV]] : (f128) -> si128
-! CHECK-KIND16: %[[CV2:.*]] = fir.convert %[[CV1]] : (si128) -> f128
-! CHECK-KIND16: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<fast> : f128
-! CHECK-KIND16: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<fast> : f128
-! CHECK-KIND16: fir.store %[[SUB]] to %[[R]] : !fir.ref<f128>
+! CHECK-FRM-KIND16: %[[A:.*]] = fir.declare{{.*}}a"
+! CHECK-FRM-KIND16: %[[P:.*]] = fir.declare{{.*}}p"
+! CHECK-FRM-KIND16: %[[R:.*]] = fir.declare{{.*}}r"
+! CHECK-FRM-KIND16: %[[A_LOAD:.*]] = fir.load %[[A]]
+! CHECK-FRM-KIND16: %[[P_LOAD:.*]] = fir.load %[[P]]
+! CHECK-FRM-KIND16: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f128
+! CHECK-FRM-KIND16: %[[CV1:.*]] = fir.convert %[[DIV]] : (f128) -> si128
+! CHECK-FRM-KIND16: %[[CV2:.*]] = fir.convert %[[CV1]] : (si128) -> f128
+! CHECK-FRM-KIND16: %[[MUL:.*]] = arith.mulf %[[CV2]], %[[P_LOAD]] fastmath<{{.*}}> : f128
+! CHECK-FRM-KIND16: %[[SUB:.*]] = arith.subf %[[A_LOAD]], %[[MUL]] fastmath<{{.*}}> : f128
+! CHECK-FRM-KIND16: fir.store %[[SUB]] to %[[R]] : !fir.ref<f128>
! CHECK-NFRM-KIND16: fir.call @_FortranAModReal16(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}}: (f128, f128, !fir.ref<i8>, i32) -> f128
r = mod(a, p)
end subroutine mod_real16
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the implementation of the -ffast-real-mod optimization flag from a negative flag (fir.no_fast_real_mod) to a positive flag (fir.fast_real_mod). The change makes -ffast-math enable fast real mod by default, while also supporting explicit control via -ffast-real-mod and -fno-fast-real-mod flags.
- Changed flag semantics from
NoFastRealModtoFastRealModthroughout the codebase - Updated test cases to verify both positive and negative flag behaviors
- Modified implementation logic to check for presence of the optimization rather than its absence
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| flang/test/Lower/Intrinsics/fast-real-mod.f90 | Updated test cases to use CHECK-FRM prefix and verify both -ffast-real-mod and -ffast-math enable the optimization |
| flang/test/Driver/fast-real-mod.f90 | Added test for -ffast-real-mod flag in driver |
| flang/lib/Optimizer/Builder/IntrinsicCall.cpp | Changed logic to check for positive fir.fast_real_mod attribute instead of negative fir.no_fast_real_mod |
| flang/lib/Frontend/FrontendActions.cpp | Updated to set fir.fast_real_mod attribute when FastRealMod option is enabled |
| flang/lib/Frontend/CompilerInvocation.cpp | Modified to set FastRealMod option when -ffast-math or -ffast-real-mod flags are present |
| flang/include/flang/Support/LangOptions.def | Renamed NoFastRealMod language option to FastRealMod |
| clang/lib/Driver/ToolChains/Flang.cpp | Added handling for -ffast-real-mod flag in driver |
| clang/include/clang/Driver/Options.td | Added definition for -ffast-real-mod flag option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,5 +1,7 @@ | |||
| ! RUN: %flang -ffast-real-mod -### -c %s 2>&1 | FileCheck %s -check-prefix CHECK-FAST-REAL-MOD | |||
| ! RUN: %flang -fno-fast-real-mod -### -c %s 2>&1 | FileCheck %s -check-prefix CHECK-NO-FAST-REAL-MOD | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be a good idea to add tests that check what happens when both -ffast-real-mod and -fno-fast-real-mod appear on the command line. The last to appear should "win".
Since these are related to ffast-math, it may be good to check that they work as expected in the presence of -ffast-math
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! That's another thing that I can add to the test.
| ! CHECK-FRM: %[[R:.*]] = fir.declare{{.*}}r" | ||
| ! CHECK-FRM: %[[A_LOAD:.*]] = fir.load %[[A]] | ||
| ! CHECK-FRM: %[[P_LOAD:.*]] = fir.load %[[P]] | ||
| ! CHECK-FRM: %[[DIV:.*]] = arith.divf %[[A_LOAD]], %[[P_LOAD]] fastmath<{{.*}}> : f32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this changed from fastmath<fast> to fastmath<{{.*}}>? Does the "argument" change depending on whether -ffast-math or -ffast-real-mod is used? If so, perhaps we should check for each of those separately.
Or was this changed because the argument is likely to change, and we don't want to have unexpected test failures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When -ffast-math is active, the fastmath tag is fast while with only -ffast-real-mod it remains at contract. I can certainly split this into two separate checks if you prefer it that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand this correctly, the -ffast-real-mod flag will result in a runtime call not being made. Since this test is only checking for that, then it's reasonable to have a wildcard match.
If we were to explicitly check for contract vs fast, it would be checking that -ffast-real-mod does not imply -ffast-math. Is that a reasonable thing to check for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I think that makes sense. I will update the test.
|
Thank you, @mjklemm. It looks okay to me, modulo Tarun's comments. If possible, can you please list the apps where you find new MOD handling preferrable or/and where using the fast MOD causes any accuracy issues. This will help others with triaging any testing failures. I think it is worth creating a small doc in |
tarunprabhu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that I had a comment pending from two days ago. Sorry for the delay
tarunprabhu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes.
The tests in Lower/Intrinsics/fast-real-mod.f90 now test that the options are correctly handled by -fc1 both with and without -ffast-math and when both -ffast-real-mod and -ffast-no-real-mod are provided. That should be checked in the driver as well. Arguably, it is better to check this in the driver since users are not expected to use -fc1 directly, but they may provide both options to the driver.
It turns out that having
-ffast-mathas the only option to control optimizations for MOD for REAL kinds (PR #160660) is too coarse-grained for some applications. Thus, this PR adds back-ffast-real-modto have more control over the optimization. The-ffast-mathflag will still enable the optimization, and-fno-fast-real-modallows one to disable it.