Author: Eugene Epshteyn Date: 2026-01-23T12:21:09-05:00 New Revision: 69eac707ed87c9f2a08dcea5947be2c4f4eec6aa
URL: https://github.com/llvm/llvm-project/commit/69eac707ed87c9f2a08dcea5947be2c4f4eec6aa DIFF: https://github.com/llvm/llvm-project/commit/69eac707ed87c9f2a08dcea5947be2c4f4eec6aa.diff LOG: [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 <[email protected]> Added: flang/test/Driver/protect-parens.f90 flang/test/Lower/HLFIR/protect-parens-arrays.f90 flang/test/Lower/HLFIR/protect-parens.f90 Modified: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Options/Options.td clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Frontend/CodeGenOptions.def flang/include/flang/Lower/LoweringOptions.def flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Lower/ConvertExprToHLFIR.cpp flang/test/Driver/fast-math.f90 Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index db0f521b73544..6798801c51142 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<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/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 8d947e0fdbabd..43727236ed5a4 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<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", @@ -7478,6 +7478,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 cd969cbb802fc..8425f8fec62a4 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 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 9f76dcc15a2a3..9cf408a5d7b34 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 4fb1d91306745..fc4975f9592eb 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 b079c9e6fa29b..0c015bc9a2f1b 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: @@ -1703,12 +1717,27 @@ class HlfirBuilder { BinaryOp<D> 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<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 = @@ -1722,14 +1751,19 @@ class HlfirBuilder { 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 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 \ diff --git a/flang/test/Driver/protect-parens.f90 b/flang/test/Driver/protect-parens.f90 new file mode 100644 index 0000000000000..f845518f678d9 --- /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 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 new file mode 100644 index 0000000000000..c5d6ec53e8140 --- /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<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 +! CHECK-FAST-NO-PROTECT: arith.mulf {{.*}} fastmath<fast> _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
