llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Cameron McInally (mcinally) <details> <summary>Changes</summary> This patch adds support for the -mrecip command line option. The parsing of this options is equivalent to Clang's and it is implemented by setting the "reciprocal-estimates" function attribute. Also move the ParseMRecip(...) function to CommonArgs, so that Flang is able to make use of it as well. --- Patch is 24.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/143418.diff 17 Files Affected: - (modified) clang/include/clang/Driver/CommonArgs.h (+5) - (modified) clang/include/clang/Driver/Options.td (+2-1) - (modified) clang/lib/Driver/ToolChains/Clang.cpp (+3-140) - (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+141) - (modified) flang/include/flang/Frontend/CodeGenOptions.h (+3) - (modified) flang/include/flang/Optimizer/Transforms/Passes.td (+3) - (modified) flang/include/flang/Tools/CrossToolHelpers.h (+3) - (modified) flang/lib/Frontend/CompilerInvocation.cpp (+2) - (modified) flang/lib/Frontend/FrontendActions.cpp (+1) - (modified) flang/lib/Optimizer/Passes/Pipelines.cpp (+2-1) - (modified) flang/lib/Optimizer/Transforms/FunctionAttr.cpp (+4) - (added) flang/test/Driver/mrecip.f90 (+27) - (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+1) - (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+5) - (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+3) - (added) mlir/test/Target/LLVMIR/Import/mrecip.ll (+9) - (added) mlir/test/Target/LLVMIR/mrecip.mlir (+8) ``````````diff diff --git a/clang/include/clang/Driver/CommonArgs.h b/clang/include/clang/Driver/CommonArgs.h index 59b68030a4d65..9d0da81b47bba 100644 --- a/clang/include/clang/Driver/CommonArgs.h +++ b/clang/include/clang/Driver/CommonArgs.h @@ -276,6 +276,11 @@ void handleVectorizeSLPArgs(const llvm::opt::ArgList &Args, StringRef ParseMPreferVectorWidthOption(clang::DiagnosticsEngine &Diags, const llvm::opt::ArgList &Args); +// Parse -mrecip. Return the Value string if well-formed. +// Otherwise, return an empty string and issue a diagnosic message if needed. +StringRef ParseMRecipOption(clang::DiagnosticsEngine &Diags, + const llvm::opt::ArgList &Args); + } // end namespace tools } // end namespace driver } // end namespace clang diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 89c63fb3397d3..3582efd7721b0 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -5474,9 +5474,10 @@ def mno_implicit_float : Flag<["-"], "mno-implicit-float">, Group<m_Group>, HelpText<"Don't generate implicit floating point or vector instructions">; def mimplicit_float : Flag<["-"], "mimplicit-float">, Group<m_Group>; def mrecip : Flag<["-"], "mrecip">, Group<m_Group>, + Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Equivalent to '-mrecip=all'">; def mrecip_EQ : CommaJoined<["-"], "mrecip=">, Group<m_Group>, - Visibility<[ClangOption, CC1Option]>, + Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, HelpText<"Control use of approximate reciprocal and reciprocal square root instructions followed by <n> iterations of " "Newton-Raphson refinement. " "<value> = ( ['!'] ['vec-'] ('rcp'|'sqrt') [('h'|'s'|'d')] [':'<n>] ) | 'all' | 'default' | 'none'">, diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 65f101ddf1d0a..df3edcc3cd6bd 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -125,145 +125,6 @@ forAllAssociatedToolChains(Compilation &C, const JobAction &JA, // } -/// This is a helper function for validating the optional refinement step -/// parameter in reciprocal argument strings. Return false if there is an error -/// parsing the refinement step. Otherwise, return true and set the Position -/// of the refinement step in the input string. -static bool getRefinementStep(StringRef In, const Driver &D, - const Arg &A, size_t &Position) { - const char RefinementStepToken = ':'; - Position = In.find(RefinementStepToken); - if (Position != StringRef::npos) { - StringRef Option = A.getOption().getName(); - StringRef RefStep = In.substr(Position + 1); - // Allow exactly one numeric character for the additional refinement - // step parameter. This is reasonable for all currently-supported - // operations and architectures because we would expect that a larger value - // of refinement steps would cause the estimate "optimization" to - // under-perform the native operation. Also, if the estimate does not - // converge quickly, it probably will not ever converge, so further - // refinement steps will not produce a better answer. - if (RefStep.size() != 1) { - D.Diag(diag::err_drv_invalid_value) << Option << RefStep; - return false; - } - char RefStepChar = RefStep[0]; - if (RefStepChar < '0' || RefStepChar > '9') { - D.Diag(diag::err_drv_invalid_value) << Option << RefStep; - return false; - } - } - return true; -} - -/// The -mrecip flag requires processing of many optional parameters. -static void ParseMRecip(const Driver &D, const ArgList &Args, - ArgStringList &OutStrings) { - StringRef DisabledPrefixIn = "!"; - StringRef DisabledPrefixOut = "!"; - StringRef EnabledPrefixOut = ""; - StringRef Out = "-mrecip="; - - Arg *A = Args.getLastArg(options::OPT_mrecip, options::OPT_mrecip_EQ); - if (!A) - return; - - unsigned NumOptions = A->getNumValues(); - if (NumOptions == 0) { - // No option is the same as "all". - OutStrings.push_back(Args.MakeArgString(Out + "all")); - return; - } - - // Pass through "all", "none", or "default" with an optional refinement step. - if (NumOptions == 1) { - StringRef Val = A->getValue(0); - size_t RefStepLoc; - if (!getRefinementStep(Val, D, *A, RefStepLoc)) - return; - StringRef ValBase = Val.slice(0, RefStepLoc); - if (ValBase == "all" || ValBase == "none" || ValBase == "default") { - OutStrings.push_back(Args.MakeArgString(Out + Val)); - return; - } - } - - // Each reciprocal type may be enabled or disabled individually. - // Check each input value for validity, concatenate them all back together, - // and pass through. - - llvm::StringMap<bool> OptionStrings; - OptionStrings.insert(std::make_pair("divd", false)); - OptionStrings.insert(std::make_pair("divf", false)); - OptionStrings.insert(std::make_pair("divh", false)); - OptionStrings.insert(std::make_pair("vec-divd", false)); - OptionStrings.insert(std::make_pair("vec-divf", false)); - OptionStrings.insert(std::make_pair("vec-divh", false)); - OptionStrings.insert(std::make_pair("sqrtd", false)); - OptionStrings.insert(std::make_pair("sqrtf", false)); - OptionStrings.insert(std::make_pair("sqrth", false)); - OptionStrings.insert(std::make_pair("vec-sqrtd", false)); - OptionStrings.insert(std::make_pair("vec-sqrtf", false)); - OptionStrings.insert(std::make_pair("vec-sqrth", false)); - - for (unsigned i = 0; i != NumOptions; ++i) { - StringRef Val = A->getValue(i); - - bool IsDisabled = Val.starts_with(DisabledPrefixIn); - // Ignore the disablement token for string matching. - if (IsDisabled) - Val = Val.substr(1); - - size_t RefStep; - if (!getRefinementStep(Val, D, *A, RefStep)) - return; - - StringRef ValBase = Val.slice(0, RefStep); - llvm::StringMap<bool>::iterator OptionIter = OptionStrings.find(ValBase); - if (OptionIter == OptionStrings.end()) { - // Try again specifying float suffix. - OptionIter = OptionStrings.find(ValBase.str() + 'f'); - if (OptionIter == OptionStrings.end()) { - // The input name did not match any known option string. - D.Diag(diag::err_drv_unknown_argument) << Val; - return; - } - // The option was specified without a half or float or double suffix. - // Make sure that the double or half entry was not already specified. - // The float entry will be checked below. - if (OptionStrings[ValBase.str() + 'd'] || - OptionStrings[ValBase.str() + 'h']) { - D.Diag(diag::err_drv_invalid_value) << A->getOption().getName() << Val; - return; - } - } - - if (OptionIter->second == true) { - // Duplicate option specified. - D.Diag(diag::err_drv_invalid_value) << A->getOption().getName() << Val; - return; - } - - // Mark the matched option as found. Do not allow duplicate specifiers. - OptionIter->second = true; - - // If the precision was not specified, also mark the double and half entry - // as found. - if (ValBase.back() != 'f' && ValBase.back() != 'd' && ValBase.back() != 'h') { - OptionStrings[ValBase.str() + 'd'] = true; - OptionStrings[ValBase.str() + 'h'] = true; - } - - // Build the output string. - StringRef Prefix = IsDisabled ? DisabledPrefixOut : EnabledPrefixOut; - Out = Args.MakeArgString(Out + Prefix + Val); - if (i != NumOptions - 1) - Out = Args.MakeArgString(Out + ","); - } - - OutStrings.push_back(Args.MakeArgString(Out)); -} - static bool shouldUseExceptionTablesForObjCExceptions(const ObjCRuntime &runtime, const llvm::Triple &Triple) { @@ -3490,7 +3351,9 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, CmdArgs.push_back(Args.MakeArgString("-fbfloat16-excess-precision=" + BFloat16ExcessPrecision)); - ParseMRecip(D, Args, CmdArgs); + StringRef Recip = ParseMRecipOption(D.getDiags(), Args); + if (!Recip.empty()) + CmdArgs.push_back(Args.MakeArgString("-mrecip=" + Recip)); // -ffast-math enables the __FAST_MATH__ preprocessor macro, but check for the // individual features enabled by -ffast-math instead of the option itself as diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 02b6e23835486..238c72dbda8a6 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -3189,3 +3189,144 @@ StringRef tools::ParseMPreferVectorWidthOption(clang::DiagnosticsEngine &Diags, return Value; } + +// This is a helper function for validating the optional refinement step +// parameter in reciprocal argument strings. Return false if there is an error +// parsing the refinement step. Otherwise, return true and set the Position +// of the refinement step in the input string. +static bool getRefinementStep(StringRef In, clang::DiagnosticsEngine &Diags, + const Arg &A, size_t &Position) { + const char RefinementStepToken = ':'; + Position = In.find(RefinementStepToken); + if (Position != StringRef::npos) { + StringRef Option = A.getOption().getName(); + StringRef RefStep = In.substr(Position + 1); + // Allow exactly one numeric character for the additional refinement + // step parameter. This is reasonable for all currently-supported + // operations and architectures because we would expect that a larger value + // of refinement steps would cause the estimate "optimization" to + // under-perform the native operation. Also, if the estimate does not + // converge quickly, it probably will not ever converge, so further + // refinement steps will not produce a better answer. + if (RefStep.size() != 1) { + Diags.Report(diag::err_drv_invalid_value) << Option << RefStep; + return false; + } + char RefStepChar = RefStep[0]; + if (RefStepChar < '0' || RefStepChar > '9') { + Diags.Report(diag::err_drv_invalid_value) << Option << RefStep; + return false; + } + } + return true; +} + +// Parse -mrecip. Return the Value string if well-formed. +// Otherwise, return an empty string and issue a diagnosic message if needed. +StringRef tools::ParseMRecipOption(clang::DiagnosticsEngine &Diags, + const ArgList &Args) { + StringRef DisabledPrefixIn = "!"; + StringRef DisabledPrefixOut = "!"; + StringRef EnabledPrefixOut = ""; + StringRef Out = ""; + + Arg *A = Args.getLastArg(options::OPT_mrecip, options::OPT_mrecip_EQ); + if (!A) + return ""; + + unsigned NumOptions = A->getNumValues(); + if (NumOptions == 0) { + // No option is the same as "all". + return "all"; + } + + // Pass through "all", "none", or "default" with an optional refinement step. + if (NumOptions == 1) { + StringRef Val = A->getValue(0); + size_t RefStepLoc; + if (!getRefinementStep(Val, Diags, *A, RefStepLoc)) + return ""; + StringRef ValBase = Val.slice(0, RefStepLoc); + if (ValBase == "all" || ValBase == "none" || ValBase == "default") { + return Val; + } + } + + // Each reciprocal type may be enabled or disabled individually. + // Check each input value for validity, concatenate them all back together, + // and pass through. + + llvm::StringMap<bool> OptionStrings; + OptionStrings.insert(std::make_pair("divd", false)); + OptionStrings.insert(std::make_pair("divf", false)); + OptionStrings.insert(std::make_pair("divh", false)); + OptionStrings.insert(std::make_pair("vec-divd", false)); + OptionStrings.insert(std::make_pair("vec-divf", false)); + OptionStrings.insert(std::make_pair("vec-divh", false)); + OptionStrings.insert(std::make_pair("sqrtd", false)); + OptionStrings.insert(std::make_pair("sqrtf", false)); + OptionStrings.insert(std::make_pair("sqrth", false)); + OptionStrings.insert(std::make_pair("vec-sqrtd", false)); + OptionStrings.insert(std::make_pair("vec-sqrtf", false)); + OptionStrings.insert(std::make_pair("vec-sqrth", false)); + + for (unsigned i = 0; i != NumOptions; ++i) { + StringRef Val = A->getValue(i); + + bool IsDisabled = Val.starts_with(DisabledPrefixIn); + // Ignore the disablement token for string matching. + if (IsDisabled) + Val = Val.substr(1); + + size_t RefStep; + if (!getRefinementStep(Val, Diags, *A, RefStep)) + return ""; + + StringRef ValBase = Val.slice(0, RefStep); + llvm::StringMap<bool>::iterator OptionIter = OptionStrings.find(ValBase); + if (OptionIter == OptionStrings.end()) { + // Try again specifying float suffix. + OptionIter = OptionStrings.find(ValBase.str() + 'f'); + if (OptionIter == OptionStrings.end()) { + // The input name did not match any known option string. + Diags.Report(diag::err_drv_unknown_argument) << Val; + return ""; + } + // The option was specified without a half or float or double suffix. + // Make sure that the double or half entry was not already specified. + // The float entry will be checked below. + if (OptionStrings[ValBase.str() + 'd'] || + OptionStrings[ValBase.str() + 'h']) { + Diags.Report(diag::err_drv_invalid_value) + << A->getOption().getName() << Val; + return ""; + } + } + + if (OptionIter->second == true) { + // Duplicate option specified. + Diags.Report(diag::err_drv_invalid_value) + << A->getOption().getName() << Val; + return ""; + } + + // Mark the matched option as found. Do not allow duplicate specifiers. + OptionIter->second = true; + + // If the precision was not specified, also mark the double and half entry + // as found. + if (ValBase.back() != 'f' && ValBase.back() != 'd' && + ValBase.back() != 'h') { + OptionStrings[ValBase.str() + 'd'] = true; + OptionStrings[ValBase.str() + 'h'] = true; + } + + // Build the output string. + StringRef Prefix = IsDisabled ? DisabledPrefixOut : EnabledPrefixOut; + Out = Args.MakeArgString(Out + Prefix + Val); + if (i != NumOptions - 1) + Out = Args.MakeArgString(Out + ","); + } + + return Out; +} diff --git a/flang/include/flang/Frontend/CodeGenOptions.h b/flang/include/flang/Frontend/CodeGenOptions.h index 61e56e51c4bbb..5c1f7ef52ca14 100644 --- a/flang/include/flang/Frontend/CodeGenOptions.h +++ b/flang/include/flang/Frontend/CodeGenOptions.h @@ -56,6 +56,9 @@ class CodeGenOptions : public CodeGenOptionsBase { // The prefered vector width, if requested by -mprefer-vector-width. std::string PreferVectorWidth; + // List of reciprocal estimate sub-options. + std::string Reciprocals; + /// List of filenames passed in using the -fembed-offload-object option. These /// are offloading binaries containing device images and metadata. std::vector<std::string> OffloadObjects; diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td index 1b1970412676d..34842f9785942 100644 --- a/flang/include/flang/Optimizer/Transforms/Passes.td +++ b/flang/include/flang/Optimizer/Transforms/Passes.td @@ -429,6 +429,9 @@ def FunctionAttr : Pass<"function-attr", "mlir::func::FuncOp"> { "module.">, Option<"unsafeFPMath", "unsafe-fp-math", "bool", /*default=*/"false", "Set the unsafe-fp-math attribute on functions in the module.">, + Option<"reciprocals", "mrecip", "std::string", /*default=*/"", + "Set the reciprocal-estimates attribute on functions in the " + "module.">, Option<"preferVectorWidth", "prefer-vector-width", "std::string", /*default=*/"", "Set the prefer-vector-width attribute on functions in the " diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h index 058024a4a04c5..337685c82af5f 100644 --- a/flang/include/flang/Tools/CrossToolHelpers.h +++ b/flang/include/flang/Tools/CrossToolHelpers.h @@ -102,6 +102,7 @@ struct MLIRToLLVMPassPipelineConfig : public FlangEPCallBacks { UnsafeFPMath = mathOpts.getAssociativeMath() && mathOpts.getReciprocalMath() && NoSignedZerosFPMath && ApproxFuncFPMath && mathOpts.getFPContractEnabled(); + Reciprocals = opts.Reciprocals; PreferVectorWidth = opts.PreferVectorWidth; if (opts.InstrumentFunctions) { InstrumentFunctionEntry = "__cyg_profile_func_enter"; @@ -127,6 +128,8 @@ struct MLIRToLLVMPassPipelineConfig : public FlangEPCallBacks { bool NoSignedZerosFPMath = false; ///< Set no-signed-zeros-fp-math attribute for functions. bool UnsafeFPMath = false; ///< Set unsafe-fp-math attribute for functions. + std::string Reciprocals = ""; ///< Set reciprocal-estimate attribute for + ///< functions. std::string PreferVectorWidth = ""; ///< Set prefer-vector-width attribute for ///< functions. bool NSWOnLoopVarInc = true; ///< Add nsw flag to loop variable increments. diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp index fb9d0ad303985..fcaf959a16a09 100644 --- a/flang/lib/Frontend/CompilerInvocation.cpp +++ b/flang/lib/Frontend/CompilerInvocation.cpp @@ -310,6 +310,8 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts, for (auto *a : args.filtered(clang::driver::options::OPT_fpass_plugin_EQ)) opts.LLVMPassPlugins.push_back(a->getValue()); + opts.Reciprocals = clang::driver::tools::ParseMRecipOption(diags, args); + opts.PreferVectorWidth = clang::driver::tools::ParseMPreferVectorWidthOption(diags, args); diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp index 012d0fdfe645f..1c8a419188b89 100644 --- a/flang/lib/Frontend/FrontendActions.cpp +++ b/flang/lib/Frontend/FrontendActions.cpp @@ -741,6 +741,7 @@ void CodeGenAction::generateLLVMIR() { config.VScaleMax = vsr->second; } + config.Reciprocals = opts.Reciprocals; config.PreferVectorWidth = opts.PreferVectorWidth; if (ci.getInvocation().getFrontendOpts().features.IsEnabled( diff --git a/flang/lib/Optimizer/Passes/Pipelines.cpp b/flang/lib/Optimizer/Passes/Pipelines.cpp index c28c8dd1730b0..70f57bdeddd3f 100644 --- a/flang/lib/Optimizer/Passes/Pipelines.cpp +++ b/flang/lib/Optimizer/Passes/Pipelines.cpp @@ -369,7 +369,8 @@ void createDefaultFIRCodeGenPassPipeline(mlir::PassManager &pm, {framePointerKind, config.InstrumentFunctionEntry, config.InstrumentFunctionExit, config.NoInfsFPMath, config.NoNaNsFPMath, config.ApproxFuncFPMath, config.NoSignedZerosFPMath, config.UnsafeFPMath, - config.PreferVectorWidth, /*tuneCPU=*/"", setNoCapture, setNoAlias})); + config.Reciprocals, config.PreferVectorWidth, /*tuneCPU=*/"", + setNoCapture, setNoAlias})); if (config.EnableOpenMP) { pm.addNestedPass<mlir::func::FuncOp>( diff --git a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp index 041aa8717d20e..5ac4ed8a93b6b 100644 --- a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp +++ b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp @@ -107,6 +107,10 @@ void FunctionAttrPass::runOnOperation() { func->setAttr( mlir::LLVM::LLVMFuncOp::getUnsafeFpMathAttrName(llvmFuncOpName), mlir::BoolAttr::get(context, true)); + if (!reciprocals.empty()) + func->setAttr( + mlir::LLVM::LLVMFuncOp::getReciprocalEstimatesAttrName(llvmFuncOpName), + mlir::StringAttr::get(context, reciprocals)); if (!preferVectorWidth.empty()) func->setAttr( mlir::LLVM::LLVMFuncOp::getPreferVectorWidthAttrName(llvmFuncOpName), diff --git a/flang/test/Driver/mrecip.f90 b/flang/test/Driver/mrecip.f90 new file mode 100644 index 0000000000000..612be1fac44df --- /dev/null +++ b/flang/test/Driver/mrecip.f90 @@ -0,0 +1,27 @@ +! Test that -mrecip[=<list>] works as expected. + +! RUN: %flang_fc1 -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-OMIT +! RUN: %flang_fc1 -mrecip -emit-llvm -o - %s 2>&1| FileChe... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/143418 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
