From 69eac707ed87c9f2a08dcea5947be2c4f4eec6aa Mon Sep 17 00:00:00 2001 From: Eugene Epshteyn Date: Fri, 23 Jan 2026 12:21:09 -0500 Subject: [PATCH] [flang] Support -f(no-)protect-parens (#170505) Driver/compiler option 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.) In the compiler, support it in code gen options and in lowering options. Hooked up lowering options with the code by @alexey-bataev that turns off reassociation transformations. Co-authored-by: Alexey Bataev --- .../clang/Basic/DiagnosticDriverKinds.td | 2 +- clang/include/clang/Options/Options.td | 5 ++- clang/lib/Driver/ToolChains/Flang.cpp | 6 +++ .../include/flang/Frontend/CodeGenOptions.def | 1 + flang/include/flang/Lower/LoweringOptions.def | 4 ++ flang/lib/Frontend/CompilerInvocation.cpp | 5 +++ flang/lib/Lower/ConvertExprToHLFIR.cpp | 42 +++++++++++++++-- flang/test/Driver/fast-math.f90 | 3 +- flang/test/Driver/protect-parens.f90 | 13 ++++++ .../Lower/HLFIR/protect-parens-arrays.f90 | 45 +++++++++++++++++++ flang/test/Lower/HLFIR/protect-parens.f90 | 32 +++++++++++++ 11 files changed, 149 insertions(+), 9 deletions(-) create mode 100644 flang/test/Driver/protect-parens.f90 create mode 100644 flang/test/Lower/HLFIR/protect-parens-arrays.f90 create mode 100644 flang/test/Lower/HLFIR/protect-parens.f90 diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index db0f521b7354..6798801c5114 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -491,7 +491,7 @@ def warn_drv_deprecated_arg_ofast : Warning< " or '-O3' to enable only conforming optimizations">, InGroup; 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; def warn_drv_deprecated_custom : Warning< diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 8d947e0fdbab..43727236ed5a 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -2929,10 +2929,10 @@ defm strict_float_cast_overflow : BoolFOption<"strict-float-cast-overflow", defm protect_parens : BoolFOption<"protect-parens", LangOpts<"ProtectParens">, DefaultFalse, - PosFlag, - NegFlag>; + NegFlag>; defm daz_ftz : SimpleMFlag<"daz-ftz", "Globally set", "Do not globally set", @@ -7478,6 +7478,7 @@ defm save_main_program : BoolOptionWithoutMarshalling<"f", "save-main-program", defm stack_arrays : BoolOptionWithoutMarshalling<"f", "stack-arrays", PosFlag, NegFlag>; + defm loop_versioning : BoolOptionWithoutMarshalling<"f", "version-loops-for-stride", PosFlag, NegFlag>; diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index cd969cbb802f..8425f8fec62a 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -203,6 +203,12 @@ 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("-fno-protect-parens"); + if (Args.hasFlag(options::OPT_funsafe_cray_pointers, options::OPT_fno_unsafe_cray_pointers, false)) { // TODO: currently passed as MLIR option diff --git a/flang/include/flang/Frontend/CodeGenOptions.def b/flang/include/flang/Frontend/CodeGenOptions.def index d5415faf06f4..05ee0e28bcaa 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 9f76dcc15a2a..9cf408a5d7b3 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 4fb1d9130674..fc4975f9592e 100644 --- a/flang/lib/Frontend/CompilerInvocation.cpp +++ b/flang/lib/Frontend/CompilerInvocation.cpp @@ -276,6 +276,10 @@ 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 = 0; + if (args.hasFlag(clang::options::OPT_fstack_arrays, clang::options::OPT_fno_stack_arrays, false)) opts.StackArrays = 1; @@ -1920,6 +1924,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/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index b079c9e6fa29..0c015bc9a2f1 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 +static bool isParenthesized(const Fortran::evaluate::Expr &expr) { + using ExprVariant = decltype(Fortran::evaluate::Expr::u); + using Parentheses = Fortran::evaluate::Parentheses; + if constexpr (Fortran::common::HasMember) { + return std::get_if(&expr.u) != nullptr; + } else { + return Fortran::common::visit( + [&](const auto &x) { return isParenthesized(x); }, expr.u); + } +} + /// Lower Designators to HLFIR. class HlfirDesignatorBuilder { private: @@ -1703,12 +1717,27 @@ private: BinaryOp binaryOp; auto left = hlfir::loadTrivialScalar(loc, builder, gen(op.left())); auto right = hlfir::loadTrivialScalar(loc, builder, gen(op.right())); + + // "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 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 = @@ -1722,14 +1751,19 @@ private: assert(right.isArray() && "must have at least one array operand"); shape = hlfir::genShape(loc, builder, right); } - auto genKernel = [&op, &left, &right, &binaryOp]( + 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 diff --git a/flang/test/Driver/fast-math.f90 b/flang/test/Driver/fast-math.f90 index e677432bc04f..22e339dc8ace 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 \ diff --git a/flang/test/Driver/protect-parens.f90 b/flang/test/Driver/protect-parens.f90 new file mode 100644 index 000000000000..f845518f678d --- /dev/null +++ b/flang/test/Driver/protect-parens.f90 @@ -0,0 +1,13 @@ +! RUN: %flang -### %s -o %t 2>&1 | FileCheck %s -check-prefix=PROTECT +! 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 -Ofast -### %s -o %t 2>&1 | FileCheck %s -check-prefix=NO-PROTECT +! RUN: %flang -Ofast -fprotect-parens -### %s -o %t 2>&1 | FileCheck %s -check-prefix=PROTECT + +! 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. +! Thus, in case of PROTECT, we don't want to have any -f[no-]protect-parens +! options. + +! PROTECT-NOT: "-f{{.*}}protect-parens" +! NO-PROTECT: "-fno-protect-parens" 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 000000000000..d6a3a193e0fa --- /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 everywhere and protect parens +! CHECK-O3: hlfir.elemental +! CHECK-O3: arith.mulf {{.*}} fastmath +! CHECK-O3: hlfir.elemental +! CHECK-O3: arith.mulf {{.*}} fastmath +! CHECK-O3: hlfir.elemental +! CHECK-O3: hlfir.no_reassoc +! CHECK-O3: hlfir.elemental +! CHECK-O3: arith.mulf {{.*}} fastmath + +! With -O3 -ffast-math, regulare computations have fastmath, but still +! protect parens, so the last multiplication is fastmath +! CHECK-FAST: hlfir.elemental +! CHECK-FAST: arith.mulf {{.*}} fastmath +! CHECK-FAST: hlfir.elemental +! CHECK-FAST: arith.mulf {{.*}} fastmath +! 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 everywhere +! (don't protect parens) +! CHECK-FAST-NO-PROTECT: hlfir.elemental +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath +! CHECK-FAST-NO-PROTECT: hlfir.elemental +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath +! 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 new file mode 100644 index 000000000000..c5d6ec53e814 --- /dev/null +++ b/flang/test/Lower/HLFIR/protect-parens.f90 @@ -0,0 +1,32 @@ +! 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 + +! With -O3, fastmath everywhere and protect parens +! CHECK-O3: arith.mulf {{.*}} fastmath +! CHECK-O3: arith.mulf {{.*}} fastmath +! CHECK-O3: hlfir.no_reassoc +! CHECK-O3: arith.mulf {{.*}} fastmath + +! With -O3 -ffast-math, regulare computations have fastmath, but still +! protect parens, so the last multiplication is fastmath +! CHECK-FAST: arith.mulf {{.*}} fastmath +! CHECK-FAST: arith.mulf {{.*}} fastmath +! CHECK-FAST: hlfir.no_reassoc +! CHECK-FAST: arith.mulf {{.*}} fastmath<{{.*}}contract + +! With -O3 -ffast-math -fno-protect-parens, fastmath everywhere +! (don't protect parens) +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath +! CHECK-FAST-NO-PROTECT: hlfir.no_reassoc +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath