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