https://github.com/eugeneepshteyn updated https://github.com/llvm/llvm-project/pull/170505
>From d6ed678c77edc1b42f927d3711b40f2835158cac Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Wed, 3 Dec 2025 11:30:10 -0500 Subject: [PATCH 01/10] Initial change from Alexey to turn off FMF --- flang/lib/Lower/ConvertExprToHLFIR.cpp | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index 1eda1f1b61355..47ba5b543752c 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -1689,12 +1689,24 @@ class HlfirBuilder { BinaryOp<D> binaryOp; auto left = hlfir::loadTrivialScalar(loc, builder, gen(op.left())); auto right = hlfir::loadTrivialScalar(loc, builder, gen(op.right())); + mlir::Value exprl = left; + mlir::Value exprr = right; + + bool noReassoc = exprl.getDefiningOp<hlfir::NoReassocOp>() || + exprr.getDefiningOp<hlfir::NoReassocOp>(); llvm::SmallVector<mlir::Value, 1> typeParams; if constexpr (R::category == Fortran::common::TypeCategory::Character) { binaryOp.genResultTypeParams(loc, builder, left, right, typeParams); } - if (rank == 0) - return binaryOp.gen(loc, builder, op.derived(), left, right); + if (rank == 0) { + auto fmfBackup = builder.getFastMathFlags(); + if (noReassoc) + builder.setFastMathFlags(fmfBackup & + ~mlir::arith::FastMathFlags::reassoc); + auto res = binaryOp.gen(loc, builder, op.derived(), left, right); + builder.setFastMathFlags(fmfBackup); + return res; + } // Elemental expression. mlir::Type elementType = @@ -1721,9 +1733,14 @@ class HlfirBuilder { // nsw is never added to operations on vector subscripts // even if -fno-wrapv is enabled. builder.setIntegerOverflowFlags(mlir::arith::IntegerOverflowFlags::none); + auto fmfBackup = builder.getFastMathFlags(); + if (noReassoc) + builder.setFastMathFlags(fmfBackup & + ~mlir::arith::FastMathFlags::reassoc); mlir::Value elemental = hlfir::genElementalOp(loc, builder, elementType, shape, typeParams, genKernel, /*isUnordered=*/true); + builder.setFastMathFlags(fmfBackup); builder.setIntegerOverflowFlags(iofBackup); fir::FirOpBuilder *bldr = &builder; getStmtCtx().attachCleanup( >From ee93d49d30afa8a3dcfe192ecd956b39f73166f3 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Wed, 3 Dec 2025 21:51:49 -0500 Subject: [PATCH 02/10] A bit of clean-up --- flang/lib/Lower/ConvertExprToHLFIR.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index 47ba5b543752c..8907d391a4847 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -1689,11 +1689,9 @@ class HlfirBuilder { BinaryOp<D> binaryOp; auto left = hlfir::loadTrivialScalar(loc, builder, gen(op.left())); auto right = hlfir::loadTrivialScalar(loc, builder, gen(op.right())); - mlir::Value exprl = left; - mlir::Value exprr = right; - bool noReassoc = exprl.getDefiningOp<hlfir::NoReassocOp>() || - exprr.getDefiningOp<hlfir::NoReassocOp>(); + bool noReassoc = left.template getDefiningOp<hlfir::NoReassocOp>() || + right.template getDefiningOp<hlfir::NoReassocOp>(); llvm::SmallVector<mlir::Value, 1> typeParams; if constexpr (R::category == Fortran::common::TypeCategory::Character) { binaryOp.genResultTypeParams(loc, builder, left, right, typeParams); >From 9ddb022a40672b51703c91fcae1b2f57d964f525 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Tue, 16 Dec 2025 21:27:46 -0500 Subject: [PATCH 03/10] Plumbing to get -f(no-)protect-parens supported on flang. This option was already supported in clang, so extended the option config to enable it in flang. Handle this option in the driver and the compiler. In the compiler, support it in code gen options and in lowering options. Not yet hooked up to the actual lowering. --- clang/include/clang/Options/Options.td | 5 +++-- clang/lib/Driver/ToolChains/Flang.cpp | 7 +++++++ flang/include/flang/Frontend/CodeGenOptions.def | 1 + flang/include/flang/Lower/LoweringOptions.def | 4 ++++ flang/lib/Frontend/CompilerInvocation.cpp | 7 +++++++ flang/test/Driver/protect-parens.f90 | 11 +++++++++++ 6 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 flang/test/Driver/protect-parens.f90 diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index c38bdd06057a2..4e4c39082c3ec 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -2907,10 +2907,10 @@ defm strict_float_cast_overflow : BoolFOption<"strict-float-cast-overflow", defm protect_parens : BoolFOption<"protect-parens", LangOpts<"ProtectParens">, DefaultFalse, - PosFlag<SetTrue, [], [ClangOption, CLOption, CC1Option], + PosFlag<SetTrue, [], [ClangOption, CLOption, CC1Option, FlangOption, FC1Option], "Determines whether the optimizer honors parentheses when " "floating-point expressions are evaluated">, - NegFlag<SetFalse>>; + NegFlag<SetFalse, [], [ClangOption, CLOption, CC1Option, FlangOption, FC1Option]>>; defm daz_ftz : SimpleMFlag<"daz-ftz", "Globally set", "Do not globally set", @@ -7437,6 +7437,7 @@ defm save_main_program : BoolOptionWithoutMarshalling<"f", "save-main-program", defm stack_arrays : BoolOptionWithoutMarshalling<"f", "stack-arrays", PosFlag<SetTrue, [], [ClangOption], "Attempt to allocate array temporaries on the stack, no matter their size">, NegFlag<SetFalse, [], [ClangOption], "Allocate array temporaries on the heap (default)">>; + defm loop_versioning : BoolOptionWithoutMarshalling<"f", "version-loops-for-stride", PosFlag<SetTrue, [], [ClangOption], "Create unit-strided versions of loops">, NegFlag<SetFalse, [], [ClangOption], "Do not create unit-strided loops (default)">>; diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index 2f5e93d139858..76d1b9d447c51 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -201,6 +201,13 @@ void Flang::addCodegenOptions(const ArgList &Args, !stackArrays->getOption().matches(options::OPT_fno_stack_arrays)) CmdArgs.push_back("-fstack-arrays"); + // -fno-protect-parens is the default for -Ofast. + if (Args.hasFlag(options::OPT_fprotect_parens, options::OPT_fno_protect_parens, + /*Default=*/!Args.hasArg(options::OPT_Ofast))) + CmdArgs.push_back("-fprotect-parens"); + else + CmdArgs.push_back("-fno-protect-parens"); + Args.addOptInFlag(CmdArgs, options::OPT_fexperimental_loop_fusion, options::OPT_fno_experimental_loop_fusion); diff --git a/flang/include/flang/Frontend/CodeGenOptions.def b/flang/include/flang/Frontend/CodeGenOptions.def index d5415faf06f47..05ee0e28bcaa6 100644 --- a/flang/include/flang/Frontend/CodeGenOptions.def +++ b/flang/include/flang/Frontend/CodeGenOptions.def @@ -40,6 +40,7 @@ CODEGENOPT(PrepareForFullLTO , 1, 0) ///< Set when -flto is enabled on the ///< compile step. CODEGENOPT(PrepareForThinLTO , 1, 0) ///< Set when -flto=thin is enabled on the ///< compile step. +CODEGENOPT(ProtectParens, 1, 1) ///< -fprotect-parens (enable parenthesis protection) CODEGENOPT(StackArrays, 1, 0) ///< -fstack-arrays (enable the stack-arrays pass) CODEGENOPT(VectorizeLoop, 1, 0) ///< Enable loop vectorization. CODEGENOPT(VectorizeSLP, 1, 0) ///< Enable SLP vectorization. diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def index 39f197d8d35c8..34a40e7c39896 100644 --- a/flang/include/flang/Lower/LoweringOptions.def +++ b/flang/include/flang/Lower/LoweringOptions.def @@ -34,6 +34,10 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0) /// On by default. ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1) +/// If true, respect parentheses in expression evaluation. +/// On by default. +ENUM_LOWERINGOPT(ProtectParens, unsigned, 1, 1) + /// If true, assume the behavior of integer overflow is defined /// (i.e. wraps around as two's complement). Off by default. ENUM_LOWERINGOPT(IntegerWrapAround, unsigned, 1, 0) diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp index b6c4e6303cdac..898a85e4fd5e5 100644 --- a/flang/lib/Frontend/CompilerInvocation.cpp +++ b/flang/lib/Frontend/CompilerInvocation.cpp @@ -276,6 +276,12 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts, clang::options::OPT_fno_debug_pass_manager, false)) opts.DebugPassManager = 1; + if (args.hasFlag(clang::options::OPT_fprotect_parens, + clang::options::OPT_fno_protect_parens, true)) + opts.ProtectParens = 1; + else + opts.ProtectParens = 0; + if (args.hasFlag(clang::options::OPT_fstack_arrays, clang::options::OPT_fno_stack_arrays, false)) opts.StackArrays = 1; @@ -1892,6 +1898,7 @@ void CompilerInvocation::setLoweringOptions() { const Fortran::common::LangOptions &langOptions = getLangOpts(); loweringOpts.setIntegerWrapAround(langOptions.getSignedOverflowBehavior() == Fortran::common::LangOptions::SOB_Defined); + loweringOpts.setProtectParens(codegenOpts.ProtectParens); Fortran::common::MathOptionsBase &mathOpts = loweringOpts.getMathOptions(); // TODO: when LangOptions are finalized, we can represent // the math related options using Fortran::commmon::MathOptionsBase, diff --git a/flang/test/Driver/protect-parens.f90 b/flang/test/Driver/protect-parens.f90 new file mode 100644 index 0000000000000..06092cfb2e429 --- /dev/null +++ b/flang/test/Driver/protect-parens.f90 @@ -0,0 +1,11 @@ +! RUN: %flang -fprotect-parens -### %s -o %t 2>&1 | FileCheck %s -check-prefix=PROTECT +! RUN: %flang -fno-protect-parens -### %s -o %t 2>&1 | FileCheck %s -check-prefix=NO-PROTECT +! RUN: %flang -### %s -o %t 2>&1 | FileCheck %s -check-prefix=DEFAULT +! RUN: %flang -Ofast -### %s -o %t 2>&1 | FileCheck %s -check-prefix=OFAST +! RUN: %flang -Ofast -fprotect-parens -### %s -o %t 2>&1 | FileCheck %s -check-prefix=OFAST-PROTECT + +! PROTECT: "-fprotect-parens" +! NO-PROTECT: "-fno-protect-parens" +! DEFAULT: "-fprotect-parens" +! OFAST: "-fno-protect-parens" +! OFAST-PROTECT: "-fprotect-parens" >From 44e0ddda447bfd243c58e3f3a374a18ccfed8656 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Tue, 16 Dec 2025 22:47:46 -0500 Subject: [PATCH 04/10] Updated -Ofast warning with -fno-protect-parens. Hooked up noReassoc behavior to ProtectParens --- clang/include/clang/Basic/DiagnosticDriverKinds.td | 2 +- flang/lib/Lower/ConvertExprToHLFIR.cpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index fca84904326c9..3b65424d28954 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -482,7 +482,7 @@ def warn_drv_deprecated_arg_ofast : Warning< " or '-O3' to enable only conforming optimizations">, InGroup<DeprecatedOFast>; def warn_drv_deprecated_arg_ofast_for_flang : Warning< - "argument '-Ofast' is deprecated; use '-O3 -ffast-math -fstack-arrays' for the same behavior," + "argument '-Ofast' is deprecated; use '-O3 -ffast-math -fstack-arrays -fno-protect-parens' for the same behavior," " or '-O3 -fstack-arrays' to enable only conforming optimizations">, InGroup<DeprecatedOFast>; def warn_drv_deprecated_custom : Warning< diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index 72480a954e3a9..d54946bc58551 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -1704,8 +1704,9 @@ class HlfirBuilder { auto left = hlfir::loadTrivialScalar(loc, builder, gen(op.left())); auto right = hlfir::loadTrivialScalar(loc, builder, gen(op.right())); - bool noReassoc = left.template getDefiningOp<hlfir::NoReassocOp>() || - right.template getDefiningOp<hlfir::NoReassocOp>(); + bool noReassoc = getConverter().getLoweringOptions().getProtectParens() && + (left.template getDefiningOp<hlfir::NoReassocOp>() || + right.template getDefiningOp<hlfir::NoReassocOp>()); llvm::SmallVector<mlir::Value, 1> typeParams; if constexpr (R::category == Fortran::common::TypeCategory::Character) { binaryOp.genResultTypeParams(loc, builder, left, right, typeParams); >From 954edc8b91112b446fd5d4e86469229bac9cb84a Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Tue, 16 Dec 2025 22:48:20 -0500 Subject: [PATCH 05/10] clang-format --- clang/lib/Driver/ToolChains/Flang.cpp | 3 ++- flang/lib/Lower/ConvertExprToHLFIR.cpp | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index 76d1b9d447c51..cc62dcb80f17a 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -202,7 +202,8 @@ void Flang::addCodegenOptions(const ArgList &Args, CmdArgs.push_back("-fstack-arrays"); // -fno-protect-parens is the default for -Ofast. - if (Args.hasFlag(options::OPT_fprotect_parens, options::OPT_fno_protect_parens, + if (Args.hasFlag(options::OPT_fprotect_parens, + options::OPT_fno_protect_parens, /*Default=*/!Args.hasArg(options::OPT_Ofast))) CmdArgs.push_back("-fprotect-parens"); else diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index d54946bc58551..4aa8382ea7624 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -1705,8 +1705,8 @@ class HlfirBuilder { auto right = hlfir::loadTrivialScalar(loc, builder, gen(op.right())); bool noReassoc = getConverter().getLoweringOptions().getProtectParens() && - (left.template getDefiningOp<hlfir::NoReassocOp>() || - right.template getDefiningOp<hlfir::NoReassocOp>()); + (left.template getDefiningOp<hlfir::NoReassocOp>() || + right.template getDefiningOp<hlfir::NoReassocOp>()); llvm::SmallVector<mlir::Value, 1> typeParams; if constexpr (R::category == Fortran::common::TypeCategory::Character) { binaryOp.genResultTypeParams(loc, builder, left, right, typeParams); >From c0a48c2705c9b12306235813bd5bed050ab7f316 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Tue, 16 Dec 2025 23:15:19 -0500 Subject: [PATCH 06/10] Lowering LIT test --- flang/test/Lower/HLFIR/protect-parens.f90 | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 flang/test/Lower/HLFIR/protect-parens.f90 diff --git a/flang/test/Lower/HLFIR/protect-parens.f90 b/flang/test/Lower/HLFIR/protect-parens.f90 new file mode 100644 index 0000000000000..b53ae2ad3b761 --- /dev/null +++ b/flang/test/Lower/HLFIR/protect-parens.f90 @@ -0,0 +1,27 @@ +! RUN: %flang_fc1 -emit-hlfir -O3 %s -o - | FileCheck %s --check-prefix=CHECK-O3 +! RUN: %flang_fc1 -emit-hlfir -O3 -ffast-math %s -o - | FileCheck %s --check-prefix=CHECK-FAST +! RUN: %flang_fc1 -emit-hlfir -O3 -ffast-math -fno-protect-parens %s -o - | FileCheck %s --check-prefix=CHECK-FAST-NO-PROTECT + +real :: a, b, c, d, e +b = 1.0 +c = 2.0 +d = 3.0 +e = 4.0 +a = b * (c * d * e) +print *, a +end + +! CHECK-O3: arith.mulf {{.*}} fastmath<contract> +! CHECK-O3: arith.mulf {{.*}} fastmath<contract> +! CHECK-O3: hlfir.no_reassoc +! CHECK-O3: arith.mulf {{.*}} fastmath<contract> + +! CHECK-FAST: arith.mulf {{.*}} fastmath<fast> +! CHECK-FAST: arith.mulf {{.*}} fastmath<fast> +! CHECK-FAST: hlfir.no_reassoc +! CHECK-FAST: arith.mulf {{.*}} fastmath<{{.*}}contract + +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<fast> +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<fast> +! CHECK-FAST-NO-PROTECT: hlfir.no_reassoc +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<fast> >From 457c1aa172ec0808d8deef49794e1bc38c85ce4d Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Tue, 16 Dec 2025 23:21:51 -0500 Subject: [PATCH 07/10] Fixed LIT test due to warning message change --- flang/test/Driver/fast-math.f90 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flang/test/Driver/fast-math.f90 b/flang/test/Driver/fast-math.f90 index e677432bc04fa..22e339dc8ace9 100644 --- a/flang/test/Driver/fast-math.f90 +++ b/flang/test/Driver/fast-math.f90 @@ -3,8 +3,7 @@ ! Check warning message for Ofast deprecation ! RUN: %flang -Ofast -### %s -o %t 2>&1 | FileCheck %s -! CHECK: warning: argument '-Ofast' is deprecated; use '-O3 -ffast-math -fstack-arrays' for the same behavior, or '-O3 -! -fstack-arrays' to enable only conforming optimizations [-Wdeprecated-ofast] +! CHECK: warning: argument '-Ofast' is deprecated; use '-O3 -ffast-math -fstack-arrays -fno-protect-parens' for the same behavior, or '-O3 -fstack-arrays' to enable only conforming optimizations [-Wdeprecated-ofast] ! -Ofast => -ffast-math -O3 -fstack-arrays ! RUN: %flang -Ofast -fsyntax-only -### %s -o %t 2>&1 \ >From a3fc7fad42362a4e564656ba3287704e6e5bd8ef Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Wed, 17 Dec 2025 08:09:35 -0500 Subject: [PATCH 08/10] Code review feedback: don't pass -fprotect-parens to the frontend, because it's default setting --- clang/lib/Driver/ToolChains/Flang.cpp | 8 +++----- flang/lib/Frontend/CompilerInvocation.cpp | 4 +--- flang/test/Driver/protect-parens.f90 | 8 +++++--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index ebad3cdaf352c..f6bb33730b5f5 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -204,11 +204,9 @@ void Flang::addCodegenOptions(const ArgList &Args, CmdArgs.push_back("-fstack-arrays"); // -fno-protect-parens is the default for -Ofast. - if (Args.hasFlag(options::OPT_fprotect_parens, - options::OPT_fno_protect_parens, - /*Default=*/!Args.hasArg(options::OPT_Ofast))) - CmdArgs.push_back("-fprotect-parens"); - else + if (!Args.hasFlag(options::OPT_fprotect_parens, + options::OPT_fno_protect_parens, + /*Default=*/!Args.hasArg(options::OPT_Ofast))) CmdArgs.push_back("-fno-protect-parens"); if (Args.hasFlag(options::OPT_funsafe_cray_pointers, diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp index 2026538e559d2..8d58b7ce01bd8 100644 --- a/flang/lib/Frontend/CompilerInvocation.cpp +++ b/flang/lib/Frontend/CompilerInvocation.cpp @@ -276,10 +276,8 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts, clang::options::OPT_fno_debug_pass_manager, false)) opts.DebugPassManager = 1; - if (args.hasFlag(clang::options::OPT_fprotect_parens, + if (!args.hasFlag(clang::options::OPT_fprotect_parens, clang::options::OPT_fno_protect_parens, true)) - opts.ProtectParens = 1; - else opts.ProtectParens = 0; if (args.hasFlag(clang::options::OPT_fstack_arrays, diff --git a/flang/test/Driver/protect-parens.f90 b/flang/test/Driver/protect-parens.f90 index 06092cfb2e429..c04e1ce98117a 100644 --- a/flang/test/Driver/protect-parens.f90 +++ b/flang/test/Driver/protect-parens.f90 @@ -4,8 +4,10 @@ ! RUN: %flang -Ofast -### %s -o %t 2>&1 | FileCheck %s -check-prefix=OFAST ! RUN: %flang -Ofast -fprotect-parens -### %s -o %t 2>&1 | FileCheck %s -check-prefix=OFAST-PROTECT -! PROTECT: "-fprotect-parens" +! Note: -fprotect-parens is not passed to the frontend, because it's the +! default. Only -fno-protect-parens is passed to turn off the default. +! PROTECT-NOT: "-f{{.*}}protect-parens" ! NO-PROTECT: "-fno-protect-parens" -! DEFAULT: "-fprotect-parens" +! DEFAULT-NOT: "-f{{.*}}protect-parens" ! OFAST: "-fno-protect-parens" -! OFAST-PROTECT: "-fprotect-parens" +! OFAST-PROTECT-NOT: "-f{{.*}}protect-parens" >From 385163572519b396bf946bff3dcb4be78083e9ff Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Wed, 17 Dec 2025 08:11:32 -0500 Subject: [PATCH 09/10] clang-format --- flang/lib/Frontend/CompilerInvocation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp index 8d58b7ce01bd8..04264a6ba805c 100644 --- a/flang/lib/Frontend/CompilerInvocation.cpp +++ b/flang/lib/Frontend/CompilerInvocation.cpp @@ -277,7 +277,7 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts, opts.DebugPassManager = 1; if (!args.hasFlag(clang::options::OPT_fprotect_parens, - clang::options::OPT_fno_protect_parens, true)) + clang::options::OPT_fno_protect_parens, true)) opts.ProtectParens = 0; if (args.hasFlag(clang::options::OPT_fstack_arrays, >From 54714cb51882bd8ebb94795f453b43b927213160 Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn <[email protected]> Date: Thu, 22 Jan 2026 00:05:54 -0500 Subject: [PATCH 10/10] Added support for array operations --- flang/lib/Lower/ConvertExprToHLFIR.cpp | 40 ++++++++++++----- .../Lower/HLFIR/protect-parens-arrays.f90 | 45 +++++++++++++++++++ flang/test/Lower/HLFIR/protect-parens.f90 | 5 +++ 3 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 flang/test/Lower/HLFIR/protect-parens-arrays.f90 diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index 4aa8382ea7624..9c688007a34d3 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -12,6 +12,7 @@ #include "flang/Lower/ConvertExprToHLFIR.h" #include "flang/Evaluate/shape.h" +#include "flang/Evaluate/tools.h" #include "flang/Lower/AbstractConverter.h" #include "flang/Lower/Allocatable.h" #include "flang/Lower/CallInterface.h" @@ -37,6 +38,19 @@ namespace { +// This was modelled after isParenthesizedVariable() +template <typename T> +static bool isParenthesized(const Fortran::evaluate::Expr<T> &expr) { + using ExprVariant = decltype(Fortran::evaluate::Expr<T>::u); + using Parentheses = Fortran::evaluate::Parentheses<T>; + if constexpr (Fortran::common::HasMember<Parentheses, ExprVariant>) { + return std::get_if<Parentheses>(&expr.u) != nullptr; + } else { + return Fortran::common::visit( + [&](const auto &x) { return isParenthesized(x); }, expr.u); + } +} + /// Lower Designators to HLFIR. class HlfirDesignatorBuilder { private: @@ -1704,9 +1718,13 @@ class HlfirBuilder { auto left = hlfir::loadTrivialScalar(loc, builder, gen(op.left())); auto right = hlfir::loadTrivialScalar(loc, builder, gen(op.right())); - bool noReassoc = getConverter().getLoweringOptions().getProtectParens() && - (left.template getDefiningOp<hlfir::NoReassocOp>() || - right.template getDefiningOp<hlfir::NoReassocOp>()); + // "A op (...)" or "(...) op A" may need to have their reassoc flag + // turned off + const bool leftIsParens = isParenthesized(op.left()); + const bool rightIsParens = isParenthesized(op.right()); + const bool noReassoc = + getConverter().getLoweringOptions().getProtectParens() && + (leftIsParens || rightIsParens); llvm::SmallVector<mlir::Value, 1> typeParams; if constexpr (R::category == Fortran::common::TypeCategory::Character) { binaryOp.genResultTypeParams(loc, builder, left, right, typeParams); @@ -1733,27 +1751,29 @@ class HlfirBuilder { assert(right.isArray() && "must have at least one array operand"); shape = hlfir::genShape(loc, builder, right); } - auto genKernel = [&op, &left, &right, &binaryOp]( + // Array operations may need to handle reassoc flag inside of + // hlfir.elemental + auto genKernel = [&op, &left, &right, &binaryOp, noReassoc]( mlir::Location l, fir::FirOpBuilder &b, mlir::ValueRange oneBasedIndices) -> hlfir::Entity { + auto fmfBackup = b.getFastMathFlags(); + if (noReassoc) + b.setFastMathFlags(fmfBackup & ~mlir::arith::FastMathFlags::reassoc); auto leftElement = hlfir::getElementAt(l, b, left, oneBasedIndices); auto rightElement = hlfir::getElementAt(l, b, right, oneBasedIndices); auto leftVal = hlfir::loadTrivialScalar(l, b, leftElement); auto rightVal = hlfir::loadTrivialScalar(l, b, rightElement); - return binaryOp.gen(l, b, op.derived(), leftVal, rightVal); + auto result = binaryOp.gen(l, b, op.derived(), leftVal, rightVal); + b.setFastMathFlags(fmfBackup); + return result; }; auto iofBackup = builder.getIntegerOverflowFlags(); // nsw is never added to operations on vector subscripts // even if -fno-wrapv is enabled. builder.setIntegerOverflowFlags(mlir::arith::IntegerOverflowFlags::none); - auto fmfBackup = builder.getFastMathFlags(); - if (noReassoc) - builder.setFastMathFlags(fmfBackup & - ~mlir::arith::FastMathFlags::reassoc); mlir::Value elemental = hlfir::genElementalOp(loc, builder, elementType, shape, typeParams, genKernel, /*isUnordered=*/true); - builder.setFastMathFlags(fmfBackup); builder.setIntegerOverflowFlags(iofBackup); fir::FirOpBuilder *bldr = &builder; getStmtCtx().attachCleanup( diff --git a/flang/test/Lower/HLFIR/protect-parens-arrays.f90 b/flang/test/Lower/HLFIR/protect-parens-arrays.f90 new file mode 100644 index 0000000000000..d6a3a193e0fae --- /dev/null +++ b/flang/test/Lower/HLFIR/protect-parens-arrays.f90 @@ -0,0 +1,45 @@ +! RUN: %flang_fc1 -emit-hlfir -O3 %s -o - | FileCheck %s --check-prefix=CHECK-O3 +! RUN: %flang_fc1 -emit-hlfir -O3 -ffast-math %s -o - | FileCheck %s --check-prefix=CHECK-FAST +! RUN: %flang_fc1 -emit-hlfir -O3 -ffast-math -fno-protect-parens %s -o - | FileCheck %s --check-prefix=CHECK-FAST-NO-PROTECT + +subroutine test_array_parens + real, dimension(10) :: a, b, c, d, e + b = 1.0 + c = 2.0 + d = 3.0 + e = 4.0 + a = b * (c * d * e) + print *, a +end subroutine + +! With -O3, fastmath<contract> everywhere and protect parens +! CHECK-O3: hlfir.elemental +! CHECK-O3: arith.mulf {{.*}} fastmath<contract> +! CHECK-O3: hlfir.elemental +! CHECK-O3: arith.mulf {{.*}} fastmath<contract> +! CHECK-O3: hlfir.elemental +! CHECK-O3: hlfir.no_reassoc +! CHECK-O3: hlfir.elemental +! CHECK-O3: arith.mulf {{.*}} fastmath<contract> + +! With -O3 -ffast-math, regulare computations have fastmath<fast>, but still +! protect parens, so the last multiplication is fastmath<contract> +! CHECK-FAST: hlfir.elemental +! CHECK-FAST: arith.mulf {{.*}} fastmath<fast> +! CHECK-FAST: hlfir.elemental +! CHECK-FAST: arith.mulf {{.*}} fastmath<fast> +! CHECK-FAST: hlfir.elemental +! CHECK-FAST: hlfir.no_reassoc +! CHECK-FAST: hlfir.elemental +! CHECK-FAST: arith.mulf {{.*}} fastmath<{{.*}}contract + +! With -O3 -ffast-math -fno-protect-parens, fastmath<fast> everywhere +! (don't protect parens) +! CHECK-FAST-NO-PROTECT: hlfir.elemental +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<fast> +! CHECK-FAST-NO-PROTECT: hlfir.elemental +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<fast> +! CHECK-FAST-NO-PROTECT: hlfir.elemental +! CHECK-FAST-NO-PROTECT: hlfir.no_reassoc +! CHECK-FAST-NO-PROTECT: hlfir.elemental +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<{{.*}}fast diff --git a/flang/test/Lower/HLFIR/protect-parens.f90 b/flang/test/Lower/HLFIR/protect-parens.f90 index b53ae2ad3b761..c5d6ec53e8140 100644 --- a/flang/test/Lower/HLFIR/protect-parens.f90 +++ b/flang/test/Lower/HLFIR/protect-parens.f90 @@ -11,16 +11,21 @@ print *, a end +! With -O3, fastmath<contract> everywhere and protect parens ! CHECK-O3: arith.mulf {{.*}} fastmath<contract> ! CHECK-O3: arith.mulf {{.*}} fastmath<contract> ! CHECK-O3: hlfir.no_reassoc ! CHECK-O3: arith.mulf {{.*}} fastmath<contract> +! With -O3 -ffast-math, regulare computations have fastmath<fast>, but still +! protect parens, so the last multiplication is fastmath<contract> ! CHECK-FAST: arith.mulf {{.*}} fastmath<fast> ! CHECK-FAST: arith.mulf {{.*}} fastmath<fast> ! CHECK-FAST: hlfir.no_reassoc ! CHECK-FAST: arith.mulf {{.*}} fastmath<{{.*}}contract +! With -O3 -ffast-math -fno-protect-parens, fastmath<fast> everywhere +! (don't protect parens) ! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<fast> ! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<fast> ! CHECK-FAST-NO-PROTECT: hlfir.no_reassoc _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
