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

mcinally · 2025-05-30T15:53:10Z

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.

llvmbot · 2025-05-30T15:53:44Z

@llvm/pr-subscribers-flang-driver
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-clang

Author: Cameron McInally (mcinally)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/142172.diff

14 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+2-1)
(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 (+159)
(modified) flang/lib/Frontend/FrontendActions.cpp (+2)
(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 --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 5ca31c253ed8f..291e9ae223805 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -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'">,
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 90a002929eff0..957f2041ffd8d 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -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
+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));
+
+  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']) {
+        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.
+  return true;
+}
+
 /// Parses all floating point related arguments and populates the
 /// CompilerInvocation accordingly.
 /// Returns false if new errors are generated.
@@ -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;
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 012d0fdfe645f..31803b27b8ceb 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -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;
diff --git a/flang/lib/Optimizer/Passes/Pipelines.cpp b/flang/lib/Optimizer/Passes/Pipelines.cpp
index 0c774eede4c9a..06eaa18fe7284 100644
--- a/flang/lib/Optimizer/Passes/Pipelines.cpp
+++ b/flang/lib/Optimizer/Passes/Pipelines.cpp
@@ -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>(
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..9ec65c0ef4dde
--- /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| 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='
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index c0324d561b77b..e5d35fa82fbce 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -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,
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 85417da798b22..1e6e60a78b37d 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -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());
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 4cc419c7cde5b..bc66625bf374e 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -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));
 
diff --git a/mlir/test/Target/LLVMIR/Import/mrecip.ll b/mlir/test/Target/LLVMIR/Import/mrecip.ll
new file mode 100644
index 0000000000000..01814cce70059
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/mrecip.ll
@@ -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" }
diff --git a/mlir/test/Target/LLVMIR/mrecip.mlir b/mlir/test/Target/LLVMIR/mrecip.mlir
new file mode 100644
index 0000000000000..e0bc66c272f6a
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/mrecip.mlir
@@ -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
+}

llvmbot · 2025-05-30T15:53:45Z

@llvm/pr-subscribers-mlir-llvm

Author: Cameron McInally (mcinally)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/142172.diff

14 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+2-1)
(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 (+159)
(modified) flang/lib/Frontend/FrontendActions.cpp (+2)
(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 --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 5ca31c253ed8f..291e9ae223805 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -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'">,
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 90a002929eff0..957f2041ffd8d 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -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
+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));
+
+  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']) {
+        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.
+  return true;
+}
+
 /// Parses all floating point related arguments and populates the
 /// CompilerInvocation accordingly.
 /// Returns false if new errors are generated.
@@ -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;
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 012d0fdfe645f..31803b27b8ceb 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -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;
diff --git a/flang/lib/Optimizer/Passes/Pipelines.cpp b/flang/lib/Optimizer/Passes/Pipelines.cpp
index 0c774eede4c9a..06eaa18fe7284 100644
--- a/flang/lib/Optimizer/Passes/Pipelines.cpp
+++ b/flang/lib/Optimizer/Passes/Pipelines.cpp
@@ -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>(
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..9ec65c0ef4dde
--- /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| 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='
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index c0324d561b77b..e5d35fa82fbce 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -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,
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 85417da798b22..1e6e60a78b37d 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -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());
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 4cc419c7cde5b..bc66625bf374e 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -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));
 
diff --git a/mlir/test/Target/LLVMIR/Import/mrecip.ll b/mlir/test/Target/LLVMIR/Import/mrecip.ll
new file mode 100644
index 0000000000000..01814cce70059
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/mrecip.ll
@@ -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" }
diff --git a/mlir/test/Target/LLVMIR/mrecip.mlir b/mlir/test/Target/LLVMIR/mrecip.mlir
new file mode 100644
index 0000000000000..e0bc66c272f6a
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/mrecip.mlir
@@ -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
+}

vzakhari

LGTM, though, I think we'd better reuse the code from Clang.cpp. flangFrontend already depends on clangDriver, so we just need to export ParseMRecip and getRefinementStep from clangDriver (and probably replace their Driver argument with a DiagnosticEngine argument, so that it works for both clang and flang).

kkwli · 2025-05-30T16:17:28Z

flang/lib/Frontend/CompilerInvocation.cpp

+/// \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


erros → errors

eugeneepshteyn · 2025-05-30T16:15:40Z

flang/lib/Frontend/CompilerInvocation.cpp

+  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));


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

eugeneepshteyn · 2025-05-30T16:17:52Z

flang/lib/Frontend/CompilerInvocation.cpp

+      if (optionStrings[valBase.str() + 'd'] ||
+          optionStrings[valBase.str() + 'h']) {


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?

eugeneepshteyn · 2025-05-30T16:33:26Z

flang/lib/Frontend/CompilerInvocation.cpp

+      out = args.MakeArgString(out + ",");
+  }
+
+  opts.Reciprocals = args.MakeArgString(out); // Handle the rest.


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

mcinally · 2025-05-31T12:29:55Z

LGTM, though, I think we'd better reuse the code from Clang.cpp. flangFrontend already depends on clangDriver, so we just need to export ParseMRecip and getRefinementStep from clangDriver (and probably replace their Driver argument with a DiagnosticEngine argument, so that it works for both clang and flang).

This seems reasonable. I'll look into whether it's possible to do.

(and probably replace their Driver argument with a DiagnosticEngine argument, so that it works for both clang and flang)

It will be interesting to see if Clang allows us to change this without changing it everywhere. Changing it everywhere seems like a heavy lift.

tarunprabhu · 2025-06-01T01:04:39Z

LGTM, though, I think we'd better reuse the code from Clang.cpp. flangFrontend already depends on clangDriver, so we just need to export ParseMRecip and getRefinementStep from clangDriver (and probably replace their Driver argument with a DiagnosticEngine argument, so that it works for both clang and flang).

This seems reasonable. I'll look into whether it's possible to do.

(and probably replace their Driver argument with a DiagnosticEngine argument, so that it works for both clang and flang)

It will be interesting to see if Clang allows us to change this without changing it everywhere. Changing it everywhere seems like a heavy lift.

If this option should be handled exactly the way it is in clang, the approach we have been using is to share the code between the two by copying it into clang/lib/Driver/ToolChains/CommonArgs.cpp. See for instance, this

mcinally · 2025-06-05T16:02:08Z

This PR has been abandoned in hopes of using the more modular solution in #142800.

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

e7c6025

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.

mcinally requested a review from tarunprabhu May 30, 2025 15:53

llvmbot added clang Clang issues not falling into any other category mlir:llvm flang:driver mlir flang Flang issues not falling into any other category flang:fir-hlfir labels May 30, 2025

mcinally requested review from jeanPerier, vzakhari, eugeneepshteyn and kiranchandramohan May 30, 2025 15:54

mcinally mentioned this pull request May 29, 2025

Request Commit Access For mcinally #141813

Open

vzakhari reviewed May 30, 2025

View reviewed changes

kkwli reviewed May 30, 2025

View reviewed changes

eugeneepshteyn reviewed May 30, 2025

View reviewed changes

erichkeane requested review from cor3ntin, AaronBallman, shafik and Endilll June 4, 2025 15:55

mcinally closed this Jun 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

mcinally commented May 30, 2025

Uh oh!

llvmbot commented May 30, 2025 •

edited

Loading

Uh oh!

llvmbot commented May 30, 2025

Uh oh!

vzakhari left a comment

Uh oh!

kkwli May 30, 2025

Uh oh!

eugeneepshteyn May 30, 2025

Uh oh!

eugeneepshteyn May 30, 2025

Uh oh!

eugeneepshteyn May 30, 2025

Uh oh!

mcinally commented May 31, 2025

Uh oh!

tarunprabhu commented Jun 1, 2025

Uh oh!

mcinally commented Jun 5, 2025

Uh oh!

Uh oh!

		if (optionStrings[valBase.str() + 'd'] \|\|
		optionStrings[valBase.str() + 'h']) {

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

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

Uh oh!

Conversation

mcinally commented May 30, 2025

Uh oh!

llvmbot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented May 30, 2025

Uh oh!

vzakhari left a comment

Choose a reason for hiding this comment

Uh oh!

kkwli May 30, 2025

Choose a reason for hiding this comment

Uh oh!

eugeneepshteyn May 30, 2025

Choose a reason for hiding this comment

Uh oh!

eugeneepshteyn May 30, 2025

Choose a reason for hiding this comment

Uh oh!

eugeneepshteyn May 30, 2025

Choose a reason for hiding this comment

Uh oh!

mcinally commented May 31, 2025

Uh oh!

tarunprabhu commented Jun 1, 2025

Uh oh!

mcinally commented Jun 5, 2025

Uh oh!

Uh oh!

llvmbot commented May 30, 2025 •

edited

Loading