Skip to content

[flang] Add support for -mrecip[=<list>] #142172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -5472,9 +5472,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'">,
Expand Down
3 changes: 3 additions & 0 deletions flang/include/flang/Frontend/CodeGenOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions flang/include/flang/Optimizer/Transforms/Passes.td
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
3 changes: 3 additions & 0 deletions flang/include/flang/Tools/CrossToolHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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.
Expand Down
159 changes: 159 additions & 0 deletions flang/lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,164 @@ static bool parseIntegerOverflowArgs(CompilerInvocation &invoc,
return true;
}

/// 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.
///
/// \param [in] in The input string
/// \param [in] a The compiler invocation arguments to parse
/// \param [out] position The position of the refinement step in input string
/// \param [out] diags DiagnosticsEngine to report erros with
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erros → errors

static bool getRefinementStep(llvm::StringRef in, const llvm::opt::Arg &a,
size_t &position,
clang::DiagnosticsEngine &diags) {
const char refinementStepToken = ':';
position = in.find(refinementStepToken);
if (position != llvm::StringRef::npos) {
llvm::StringRef option = a.getOption().getName();
llvm::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(clang::diag::err_drv_invalid_value) << option << refStep;
return false;
}
char refStepChar = refStep[0];
if (refStepChar < '0' || refStepChar > '9') {
diags.Report(clang::diag::err_drv_invalid_value) << option << refStep;
return false;
}
}
return true;
}

/// Parses all -mrecip=<list> arguments and populates the
/// CompilerInvocation accordingly. Returns false if new errors are generated.
///
/// \param [out] invoc Stores the processed arguments
/// \param [in] args The compiler invocation arguments to parse
/// \param [out] diags DiagnosticsEngine to report erros with
static bool parseMRecip(CompilerInvocation &invoc, llvm::opt::ArgList &args,
clang::DiagnosticsEngine &diags) {
llvm::StringRef disabledPrefixIn = "!";
llvm::StringRef disabledPrefixOut = "!";
llvm::StringRef enabledPrefixOut = "";
llvm::StringRef out = "";
Fortran::frontend::CodeGenOptions &opts = invoc.getCodeGenOpts();

const llvm::opt::Arg *a =
args.getLastArg(clang::driver::options::OPT_mrecip,
clang::driver::options::OPT_mrecip_EQ);
if (!a)
return true;

unsigned numOptions = a->getNumValues();
if (numOptions == 0) {
// No option is the same as "all".
opts.Reciprocals = "all";
return true;
}

// Pass through "all", "none", or "default" with an optional refinement step.
if (numOptions == 1) {
llvm::StringRef val = a->getValue(0);
size_t refStepLoc;
if (!getRefinementStep(val, *a, refStepLoc, diags))
return false;
llvm::StringRef valBase = val.slice(0, refStepLoc);
if (valBase == "all" || valBase == "none" || valBase == "default") {
opts.Reciprocals = args.MakeArgString(val);
return true;
}
}

// 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));
Comment on lines +1336 to +1347
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, it may be cleaner to do something like this: optionStrings["divd"] = false;, etc.


for (unsigned i = 0; i != numOptions; ++i) {
llvm::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, *a, refStep, diags))
return false;

llvm::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(clang::diag::err_drv_invalid_value)
<< a->getOption().getName() << val;
return false;
}
// 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']) {
Comment on lines +1375 to +1376
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using operator [] on a map has a side-effect of creating an entry, if it's not already in the map. Is this intended here?

diags.Report(clang::diag::err_drv_invalid_value)
<< a->getOption().getName() << val;
return false;
}
}

if (optionIter->second == true) {
// Duplicate option specified.
diags.Report(clang::diag::err_drv_invalid_value)
<< a->getOption().getName() << val;
return false;
}

// 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.
llvm::StringRef prefix = isDisabled ? disabledPrefixOut : enabledPrefixOut;
out = args.MakeArgString(out + prefix + val);
if (i != numOptions - 1)
out = args.MakeArgString(out + ",");
}

opts.Reciprocals = args.MakeArgString(out); // Handle the rest.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't found any issues with the string memory ownership, but just to be sure maybe run this under valgrind.

return true;
}

/// Parses all floating point related arguments and populates the
/// CompilerInvocation accordingly.
/// Returns false if new errors are generated.
Expand Down Expand Up @@ -1398,6 +1556,7 @@ static bool parseLangOptionsArgs(CompilerInvocation &invoc,

success &= parseIntegerOverflowArgs(invoc, args, diags);
success &= parseFloatingPointArgs(invoc, args, diags);
success &= parseMRecip(invoc, args, diags);
success &= parseVScaleArgs(invoc, args, diags);

return success;
Expand Down
2 changes: 2 additions & 0 deletions flang/lib/Frontend/FrontendActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,8 @@ void CodeGenAction::generateLLVMIR() {

config.PreferVectorWidth = opts.PreferVectorWidth;

config.Reciprocals = opts.Reciprocals;

if (ci.getInvocation().getFrontendOpts().features.IsEnabled(
Fortran::common::LanguageFeature::OpenMP))
config.EnableOpenMP = true;
Expand Down
3 changes: 2 additions & 1 deletion flang/lib/Optimizer/Passes/Pipelines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,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>(
Expand Down
4 changes: 4 additions & 0 deletions flang/lib/Optimizer/Transforms/FunctionAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
27 changes: 27 additions & 0 deletions flang/test/Driver/mrecip.f90
Original file line number Diff line number Diff line change
@@ -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| FileCheck %s --check-prefix=CHECK-NOARG
! RUN: %flang_fc1 -mrecip=all -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-ALL
! RUN: %flang_fc1 -mrecip=none -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-NONE
! RUN: %flang_fc1 -mrecip=default -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-DEF
! RUN: %flang_fc1 -mrecip=divd,divf,divh,vec-divd,vec-divf,vec-divh,sqrtd,sqrtf,sqrth,vec-sqrtd,vec-sqrtf,vec-sqrth -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-POS
! RUN: %flang_fc1 -mrecip=!divd,!divf,!divh,!vec-divd,!vec-divf,!vec-divh,!sqrtd,!sqrtf,!sqrth,!vec-sqrtd,!vec-sqrtf,!vec-sqrth
! -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-NEG
! RUN: %flang_fc1 -mrecip=!divd,divf,!divh,sqrtd,!sqrtf,sqrth -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-MIX
! RUN: not %flang_fc1 -mrecip=xxx -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-INV
! RUN: not %flang_fc1 -mrecip=divd,divd -emit-llvm -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-DUP

subroutine func
end subroutine func

! CHECK-OMIT-NOT: attributes #0 = { "reciprocal-estimates"={{.*}} }
! CHECK-NOARG: attributes #0 = { "reciprocal-estimates"="all" }
! CHECK-ALL: attributes #0 = { "reciprocal-estimates"="all" }
! CHECK-NONE: attributes #0 = { "reciprocal-estimates"="none" }
! CHECK-DEF: attributes #0 = { "reciprocal-estimates"="default" }
! CHECK-POS: attributes #0 = { "reciprocal-estimates"="divd,divf,divh,vec-divd,vec-divf,vec-divh,sqrtd,sqrtf,sqrth,vec-sqrtd,vec-sqrtf,vec-sqrth" }
! CHECK-NEG: attributes #0 = { "reciprocal-estimates"="!divd,!divf,!divh,!vec-divd,!vec-divf,!vec-divh,!sqrtd,!sqrtf,!sqrth,!vec-sqrtd,!vec-sqrtf,!vec-sqrth" }
! CHECK-MIX: attributes #0 = { "reciprocal-estimates"="!divd,divf,!divh,sqrtd,!sqrtf,sqrth" }
! CHECK-INV: error: invalid value 'xxx' in 'mrecip='
! CHECK-DUP: error: invalid value 'divd' in 'mrecip='
1 change: 1 addition & 0 deletions mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -1895,6 +1895,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
OptionalAttr<StrAttr>:$tune_cpu,
OptionalAttr<StrAttr>:$prefer_vector_width,
OptionalAttr<LLVM_TargetFeaturesAttr>:$target_features,
OptionalAttr<StrAttr>:$reciprocal_estimates,
OptionalAttr<BoolAttr>:$unsafe_fp_math,
OptionalAttr<BoolAttr>:$no_infs_fp_math,
OptionalAttr<BoolAttr>:$no_nans_fp_math,
Expand Down
5 changes: 5 additions & 0 deletions mlir/lib/Target/LLVMIR/ModuleImport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2640,6 +2640,11 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func,
attr.isStringAttribute())
funcOp.setPreferVectorWidth(attr.getValueAsString());

if (llvm::Attribute attr = func->getFnAttribute("reciprocal-estimates");
attr.isStringAttribute())
funcOp.setReciprocalEstimatesAttr(
StringAttr::get(context, attr.getValueAsString()));

if (llvm::Attribute attr = func->getFnAttribute("unsafe-fp-math");
attr.isStringAttribute())
funcOp.setUnsafeFpMath(attr.getValueAsBool());
Expand Down
3 changes: 3 additions & 0 deletions mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1557,6 +1557,9 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
getLLVMContext(), attr->getMinRange().getInt(),
attr->getMaxRange().getInt()));

if (auto reciprocalEstimates = func.getReciprocalEstimates())
llvmFunc->addFnAttr("reciprocal-estimates", *reciprocalEstimates);

if (auto unsafeFpMath = func.getUnsafeFpMath())
llvmFunc->addFnAttr("unsafe-fp-math", llvm::toStringRef(*unsafeFpMath));

Expand Down
9 changes: 9 additions & 0 deletions mlir/test/Target/LLVMIR/Import/mrecip.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s

; CHECK-LABEL: llvm.func @reciprocal_estimates()
; CHECK-SAME: reciprocal_estimates = "all"
define void @reciprocal_estimates() #0 {
ret void
}

attributes #0 = { "reciprocal-estimates" = "all" }
8 changes: 8 additions & 0 deletions mlir/test/Target/LLVMIR/mrecip.mlir
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s

// CHECK: define void @reciprocal_estimates() #[[ATTRS:.*]] {
// CHECK: attributes #[[ATTRS]] = { "reciprocal-estimates"="all" }

llvm.func @reciprocal_estimates() attributes {reciprocal_estimates = "all"} {
llvm.return
}
Loading