make reciprocal estimate code generation more flexible by adding command-line options

[spatel]

Summary

We need separation of scalar and vector reciprocal codegen to handle an -mrecip clang flag that provides equal functionality to gcc's:
https://gcc.gnu.org/onlinedocs/gcc-4.9.2/gcc/i386-and-x86-64-Options.html#index-mrecip_003dopt-1627

This patch adds a Target class for processing all of the recip codegen possibilities. The x86 backend is updated to use the new functionality.

[hfinkel]

Whether or not we're allowed to use estimates for operations that are normally required to be exact, div and sqrt, are not really target features. They're much closer, conceptually, to UnsafeFPMath and friends. Plus, we need these same options for other backends (PowerPC, for example). I'd like to see flags for these added to include/llvm/Target/TargetOptions.h (along with UnsafeFPMath) so that we can handle them in a uniform way.

[spatel]

Hi Hal -

Thanks for the quick feedback. I'm looking at the PPC variations that gcc supports. I'm thinking that we'll need an enum with bitmasks to cover all the potential variations. Just making sure that I'm not going into the weeds...

Also (if maintaining compatibility with gcc is a goal), we'll still need to accept and process different flags per arch.

[hfinkel]

In D8982#155207, @spatel wrote:

Hi Hal -

Thanks for the quick feedback. I'm looking at the PPC variations that gcc supports. I'm thinking that we'll need an enum with bitmasks to cover all the potential variations. Just making sure that I'm not going into the weeds...

I think that sounds right.

Also (if maintaining compatibility with gcc is a goal), we'll still need to accept and process different flags per arch.

Not exactly in that sense. If we accept a superset of the options that gcc supports because we apply them to targets/variants that gcc does not, that's fine. This seems pretty target independent, and I'd rather handle it in a uniform way if possible.

[spatel]

After thinking it over, I decided that a simple enum of reciprocal operation bools wasn't going to do the job. In addition to wanting to enable specific ops on specific data types, we have users asking to customize the number of N-R refinements calculated...and they want different counts based on the type of op.

The best way to achieve this level of flexibility (and further customization requests that are sure to follow) is with a 'TargetRecip' class. So that's what I'm proposing to add in this updated patch.

From the llc command-line, you can now do something like this:
$ llc -recip=vec-divf,sqrtd.2

That translates to: allow vector float division and scalar double square root codegen. Use the target default setting for N-R refinement steps for the div but override the refinement steps for the sqrt to be '2'.

To minimize change in the updated x86 backend, I've defaulted to disabling all recip codegen, but for enabled operations, we use 1 N-R step.

This means only users that were targeting AMD Jaguar will see a functional change from this patch (that chip had defaulted both sqrt and div codegen for scalars and vectors on). We can easily change the x86 target defaults in a follow-on patch. We'll also want to update the PowerPC and R600 backends to use this new command-line functionality.

I wanted to add more testing for the llc parameter parsing, but I'm not finding a way to do it. If there's a way to do that with a generic target, please let me know.

I have a dependent clang driver / front-end patch to pass similar command-line params through to the backend in progress.

[spatel]

Patch updated: minor cleanup to spacing, includes, interface, comments.

[spatel]

Ping.

[spatel]

Ping * 2.

[spatel]

Ping * 3.

[spatel]

Ping * 4.

[hfinkel]

include/llvm/Target/TargetOptions.h

216

reciprocal estimate -> reciprocal-estimate

include/llvm/Target/TargetRecip.h

35

To reduce confusion, I think it would be better to have a naming prefix on these. TailCallKind, for example, uses TCK_. Let's stick RO_ on these (including the INVALID one).

35

If you're using INVALID for the "number of", please name it NUM_RECIP_OPS (or similar).

49

Let's not use INVALID for "all". You can add an All to the enum (it can even have the same value as INVALID).

lib/Target/TargetRecip.cpp

59

These strings are user-provided, we can't assert on invalid inputs.

149

Hrmm, there could be duplicates. Just parse them in order (users may provide duplicates).

[spatel]

include/llvm/Target/TargetRecip.h

35

With a named enum, I was expecting that users always have to use the prefix "RecipOps::" like I did in the x86 code.

But I see that's not done with TailCallKind...

[spatel]

include/llvm/Target/TargetOptions.h

216

Fixed.

include/llvm/Target/TargetRecip.h

35

Fixed.

35

Fixed.

49

Fixed - added an RO_All that is equal to RO_NUM_RECIP_OPS.

lib/Target/TargetRecip.cpp

59

Fixed - replaced with 'report_fatal_error'.

149

Fixed. Still not sure if there's a decent way to test option parsing for llc, so there are very likely malformed input parsing bugs here.

[spatel]

Patch updated based on feedback from Hal - thanks!

[spatel]

Patch updated:

1. Use '!' to indicate disabled (match change in D8989)
2. Use array of strings as keys to a map instead of enum + loosely-coupled array of recip parameters.

[hfinkel]

include/llvm/Target/TargetRecip.h

67

function names should start with a lower-case letter.

lib/Target/TargetRecip.cpp

50

Function names start with a lower-case letter.

113

DisabledPrefix (this is not a macro, don't name it like one)

133

Extra space?

137

Please also try with the 'd' suffix.

[spatel]

include/llvm/Target/TargetRecip.h

67

Fixed.

lib/Target/TargetRecip.cpp

50

Fixed.

113

Fixed.

133

Fixed. Nice catch. :)

137

If we matched a 'd' entry but failed 'f', that would be an assertion failure given the logic below. Let me know if you were thinking of something else.

[spatel]

Patch updated based on Hal's feedback:

1. Fixed function names to start with lowercase
2. Fixed variable names to not be all caps
3. Removed extra space
4. Added assert for 'f' and 'd' suffix matching

[hfinkel]

lib/Target/X86/X86TargetMachine.cpp

112

Why false? Do you want a target feature here?

[spatel]

lib/Target/X86/X86TargetMachine.cpp

112

We had target features to control these, but I think it would be better to behave like gcc unless we have reason to diverge.

That said, this does not match gcc behavior yet; that would be my next patch. For x86 at least, we would turn the following on by default when using -ffast-math:
sqrt
vec-sqrt
vec-div

I didn't set these defaults in this patch because it would change -ffast-math codegen for all CPUs other than btver2 (which had the recip codegen enabled for all eligible x86 recip types via target features).

[hfinkel]

lib/Target/X86/X86TargetMachine.cpp

112

I thought that getRsqrtEstimate, etc. were only called when fast-math is on?

[spatel]

lib/Target/X86/X86TargetMachine.cpp

112

That's correct; everything is gated by -ffast-math or an unsafe algebra equivalent. Recip-est codegen should only be active after that check.

With gcc, once you have -ffast-math, you also get -mrecip=sqrt,vec-sqrt,vec-div by default. AFAIK, that is independent of arch or CPU subtarget.

The lone exception is 'div' (scalar division). Estimating scalar div on x86 (no FMA until recently) breaks a lot of real world code, so I want to keep that off by default (and again match gcc default behavior).

[hfinkel]

lib/Target/X86/X86TargetMachine.cpp

112

But you turn them all off here, right? So that does not match gcc. Please add a comment here explaining what is going on.

[spatel]

lib/Target/X86/X86TargetMachine.cpp

112

Right - my intent was to separate the structural change from the functional change as much as possible.

We don't currently generate any reciprocal estimates for x86 with -ffast-math except when targeting btver2. So all x86 codegen should be identical after this patch except in the case where someone has specified -mcpu=btver2 -ffast-math.

I'll send the patch to change this default behavior to match GCC defaults as soon as possible, but I wanted to keep that separate in case anyone does not agree that we should change that default behavior. PPC and ARM recip patches would also be independent but similar (assuming they want to match GCC too).

[hfinkel]

LGTM.

lib/Target/X86/X86TargetMachine.cpp

112

Okay, but please add a comment explaining the current state of things, with a TODO, here (maybe the follow-up with happen soon, but in case there's trouble, the code will be clear in the mean time).

[spatel]

Patch updated:
Added TODO comment to explain why x86 reciprocal estimate is defaulted to 'off' and the reason to change that default soon.

include/llvm/CodeGen/CommandFlags.h

 #include "llvm/IR/Module.h"
 #include "llvm/MC/MCTargetOptionsCommandFlags.h"
 #include "llvm//MC/SubtargetFeature.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
+#include "llvm/Target/TargetRecip.h"
 #include <string>
 using namespace llvm;
 
 cl::opt<std::string>
 MArch("march", cl::desc("Architecture to generate code for (see --version)"));
 
 cl::opt<std::string>
 MCPU("mcpu",
FuseFPOps("fp-contract",
               clEnumValN(FPOpFusion::Fast, "fast",
                          "Fuse FP ops whenever profitable"),
               clEnumValN(FPOpFusion::Standard, "on",
                          "Only fuse 'blessed' FP ops."),
               clEnumValN(FPOpFusion::Strict, "off",
                          "Only fuse FP ops when the result won't be effected."),
               clEnumValEnd));
 
+cl::list<std::string>
+ReciprocalOps("recip",
+       cl::CommaSeparated,
+       cl::desc("Choose reciprocal operation types and parameters."),
+       cl::value_desc("all,none,default,divf,vec-sqrtd,vec-divd:0,sqrtf:9..."));
+
 cl::opt<bool>
 DontPlaceZerosInBSS("nozero-initialized-in-bss",
               cl::desc("Don't place zero-initialized symbols into bss section"),
               cl::init(false));
 
 cl::opt<bool>
 EnableGuaranteedTailCallOpt("tailcallopt",
   cl::desc("Turn fastcc calls into tail calls by (potentially) changing ABI."),

 
 // Common utility function tightly tied to the options listed here. Initializes
 // a TargetOptions object with CodeGen flags and returns it.
 static inline TargetOptions InitTargetOptionsFromCodeGenFlags() {
   TargetOptions Options;
   Options.LessPreciseFPMADOption = EnableFPMAD;
   Options.NoFramePointerElim = DisableFPElim;
   Options.AllowFPOpFusion = FuseFPOps;
+  Options.Reciprocals = ReciprocalOps;
   Options.UnsafeFPMath = EnableUnsafeFPMath;
   Options.NoInfsFPMath = EnableNoInfsFPMath;
   Options.NoNaNsFPMath = EnableNoNaNsFPMath;
   Options.HonorSignDependentRoundingFPMathOption =
       EnableHonorSignDependentRoundingFPMath;
   if (FloatABIForCalls != FloatABI::Default)
     Options.FloatABIType = FloatABIForCalls;
   Options.NoZerosInBSS = DontPlaceZerosInBSS;

include/llvm/Target/TargetOptions.h

 // This file defines command line option flags that are shared across various
 // targets.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TARGET_TARGETOPTIONS_H
 #define LLVM_TARGET_TARGETOPTIONS_H
 
+#include "llvm/Target/TargetRecip.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include <string>
 
 namespace llvm {
   class MachineFunction;
   class StringRef;
 
   // Possible float ABI settings. Used with FloatABIType in TargetOptions.h.
TargetOptions()
           NoZerosInBSS(false),
           GuaranteedTailCallOpt(false),
           DisableTailCalls(false), StackAlignmentOverride(0),
           EnableFastISel(false), PositionIndependentExecutable(false),
           UseInitArray(false), DisableIntegratedAS(false),
           CompressDebugSections(false), FunctionSections(false),
           DataSections(false), UniqueSectionNames(true), TrapUnreachable(false),
           TrapFuncName(), FloatABIType(FloatABI::Default),
-          AllowFPOpFusion(FPOpFusion::Standard), JTType(JumpTable::Single),
+          AllowFPOpFusion(FPOpFusion::Standard), Reciprocals(),
+          JTType(JumpTable::Single),
           ThreadModel(ThreadModel::POSIX) {}
 
     /// PrintMachineCode - This flag is enabled when the -print-machineinstrs
     /// option is specified on the command line, and should enable debugging
     /// output from the code generator.
     unsigned PrintMachineCode : 1;
 
     /// NoFramePointerElim - This flag is enabled when the -disable-fp-elim is
public:
     /// precision won't effect the result.
     ///
     /// Note: This option only controls formation of fused ops by the
     /// optimizers.  Fused operations that are explicitly specified (e.g. FMA
     /// via the llvm.fma.* intrinsic) will always be honored, regardless of
     /// the value of this option.
     FPOpFusion::FPOpFusionMode AllowFPOpFusion;
 
+    /// This class encapsulates options for reciprocal-estimate code generation.
+    TargetRecip Reciprocals;
+    
     /// JTType - This flag specifies the type of jump-instruction table to
     /// create for functions that have the jumptable attribute.
     JumpTable::JumpTableType JTType;
 
     /// ThreadModel - This flag specifies the type of threading model to assume
     /// for things like atomics
     ThreadModel::Model ThreadModel;
 
return
     ARE_EQUAL(StackAlignmentOverride) &&
     ARE_EQUAL(EnableFastISel) &&
     ARE_EQUAL(PositionIndependentExecutable) &&
     ARE_EQUAL(UseInitArray) &&
     ARE_EQUAL(TrapUnreachable) &&
     ARE_EQUAL(TrapFuncName) &&
     ARE_EQUAL(FloatABIType) &&
     ARE_EQUAL(AllowFPOpFusion) &&
+    ARE_EQUAL(Reciprocals) &&
     ARE_EQUAL(JTType) &&
     ARE_EQUAL(ThreadModel) &&
     ARE_EQUAL(MCOptions);
 #undef ARE_EQUAL
 }
 
 inline bool operator!=(const TargetOptions &LHS,
                        const TargetOptions &RHS) {
   return !(LHS == RHS);
 }
 
 } // End llvm namespace
 
 #endif

include/llvm/Target/TargetRecip.h

+//===--------------------- llvm/Target/TargetRecip.h ------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This class is used to customize machine-specific reciprocal estimate code
+// generation in a target-independent way.
+// If a target does not support operations in this specification, then code
+// generation will default to using supported operations.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TARGET_TARGETRECIP_H
+#define LLVM_TARGET_TARGETRECIP_H
+
+#include <vector>
+#include <string>
+
+namespace llvm {
+
+enum RecipOps {
+  RO_DivF = 0,     // division,    float,  scalar
+  RO_VecDivF,      // division,    float,  vector
+  RO_DivD,         // division,    double, scalar
+  RO_VecDivD,      // division,    double, vector
+  RO_SqrtF,        // square root, float,  scalar
+  RO_VecSqrtF,     // square root, float,  vector
+  RO_SqrtD,        // square root, double, scalar
+  RO_VecSqrtD,     // square root, double, vector
+  RO_All,
+  RO_NUM_RECIP_OPS = RO_All
+};
+
+class TargetRecip {
+public:
+  TargetRecip();
+
+  /// Initialize all or part of the operations from command-line options or
+  /// encoded strings.
+  TargetRecip(const std::vector<std::string> &Args);
+
+  virtual ~TargetRecip();
+  
+  /// Set whether a particular reciprocal operation is enabled and how many
+  /// refinement steps are needed when using it. Use the 'RO_All' value
+  /// to set enablement and refinement steps for all operations.
+  void setDefaults(RecipOps Op, bool Enable, unsigned RefSteps);
+
+  /// Return true if the reciprocal operation has been enabled by default or
+  /// from the command-line. Return false if the operations has been disabled
+  /// by default or from the command-line.
+  bool isEnabled(RecipOps Op) const;
+
+  /// Return the number of iterations necessary to refine the
+  /// the result of a machine instruction for the given reciprocal operation.
+  unsigned getRefinementSteps(RecipOps Op) const;
+
+  bool operator==(const TargetRecip &Other) const;
+
+private:
+  int8_t Enabled[RO_NUM_RECIP_OPS];
+  int8_t RefinementSteps[RO_NUM_RECIP_OPS];
+  
+  bool ParseGlobalParams(const std::string &Arg);
+  void ParseIndividualParams(const std::string &Arg);
+};
+
+} // End llvm namespace
+
+#endif

lib/Target/CMakeLists.txt

 list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 
 add_llvm_library(LLVMTarget
   Target.cpp
   TargetIntrinsicInfo.cpp
   TargetLoweringObjectFile.cpp
   TargetMachine.cpp
   TargetMachineC.cpp
+  TargetRecip.cpp
   TargetSubtargetInfo.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Target
   )
 
 foreach(t ${LLVM_TARGETS_TO_BUILD})
   message(STATUS "Targeting ${t}")
   add_subdirectory(${t})
 endforeach()

lib/Target/TargetRecip.cpp

+//===-------------------------- TargetRecip.cpp ---------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This class is used to customize machine-specific reciprocal estimate code
+// generation in a target-independent way.
+// If a target does not support operations in this specification, then code
+// generation will default to using supported operations.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Target/TargetRecip.h"
+
+using namespace llvm;
+
+// These must be in the same order as the corresponding enum values.
+const char *ArgStrings[] = {
+  "divf",
+  "vec-divf",
+  "divd",
+  "vec-divd",
+  "sqrtf",
+  "vec-sqrtf",
+  "sqrtd",
+  "vec-sqrtd"
+};
+
+// The uninitialized state is needed for the enablement bits and refinement
+// steps because custom settings may arrive via the command-line before target
+// defaults are set.
+enum {
+  Uninitialized = -1
+};
+
+TargetRecip::TargetRecip() {
+  for (int i = 0; i != RO_NUM_RECIP_OPS; ++i) {
+    Enabled[i] = Uninitialized;
+    RefinementSteps[i] = Uninitialized;
+  }
+}
+
+static bool ParseRefinementStep(const std::string &In, size_t &Position,
+                                uint8_t &Value) {
+  const char REF_STEP_TOKEN = ':';
+  Position = In.find(REF_STEP_TOKEN);
+  if (Position == std::string::npos)
+    return false;
+  
+  std::string RefStepString = In.substr(Position + 1);
+  // Allow exactly one numeric character for the additional refinement
+  // step parameter.
+  if (RefStepString.length() == 1) {
+    char RefStepChar = RefStepString[0];
+    if (RefStepChar >= '0' && RefStepChar <= '9') {
+      Value = RefStepChar - '0';
+      return true;
+    }
+  }
+  report_fatal_error("Invalid refinement step for -recip.");
+}
+
+bool TargetRecip::ParseGlobalParams(const std::string &Arg) {
+  bool Enable;
+  bool UseDefaults;
+  if (Arg.find("all") == 0) {
+    UseDefaults = false;
+    Enable = true;
+  } else if (Arg.find("none") == 0) {
+    UseDefaults = false;
+    Enable = false;
+  } else if (Arg.find("default") == 0) {
+    UseDefaults = true;
+  } else {
+    // Any other string is invalid or an individual setting.
+    return false;
+  }
+  
+  // All enable values will be initialized to target defaults if 'default' was
+  // specified.
+  if (!UseDefaults)
+    for (int i = 0; i != RO_NUM_RECIP_OPS; ++i)
+      Enabled[i] = Enable;
+
+  size_t RefPos;
+  uint8_t RefSteps;
+  if (ParseRefinementStep(Arg, RefPos, RefSteps)) {
+    // Custom refinement count was specified with all, none, or default.
+    for (int i = 0; i != RO_NUM_RECIP_OPS; ++i)
+      RefinementSteps[i] = RefSteps;
+  }
+  return true;
+}
+
+void TargetRecip::ParseIndividualParams(const std::string &Arg) {
+  std::string ArgSub = Arg;
+
+  // Each reciprocal type may be enabled ('+') or disabled ('-') individually.
+  bool IsEnabled;
+  if (Arg[0] == '+') {
+    ArgSub = Arg.substr(1);
+    IsEnabled = true;
+  } else if (Arg[0] == '-') {
+    ArgSub = Arg.substr(1);
+    IsEnabled = false;
+  } else {
+    // If no plus or minus, default to plus.
+    IsEnabled = true;
+  }
+  
+  // Look for an optional setting of the number of refinement steps needed
+  // for this type of reciprocal operation.
+  size_t RefPos;
+  uint8_t RefSteps;
+  std::string RefStepString;
+  if (ParseRefinementStep(ArgSub, RefPos, RefSteps)) {
+    // Split the string for further processing.
+    RefStepString = ArgSub.substr(RefPos + 1);
+    ArgSub = ArgSub.substr(0, RefPos);
+  }
+  
+  // Find the reciprocal operation corresponding to this string value.
+  RecipOps Op = RO_NUM_RECIP_OPS;
+  for (int i = 0; i != RO_NUM_RECIP_OPS; ++i) {
+    if (ArgSub == ArgStrings[i]) {
+      Op = (RecipOps)i;
+      break;
+    }
+  }
+  
+  if (Op == RO_NUM_RECIP_OPS)
+    report_fatal_error("Invalid option for -recip.");
+  
+  
+  // Set whether this operation is being enabled or disabled, and optionally
+  // set the number of refinement steps for this operation.
+  Enabled[Op] = IsEnabled;
+  if (!RefStepString.empty()) {
+    RefinementSteps[Op] = RefSteps;
+  }
+}
+
+TargetRecip::TargetRecip(const std::vector<std::string> &Args) :
+  TargetRecip::TargetRecip() {
+  unsigned NumArgs = Args.size();
+
+  // Check if "all", "default", or "none" was specified.
+  if (NumArgs == 1 && ParseGlobalParams(Args[0]))
+    return;
+
+  for (unsigned i = 0; i != NumArgs; ++i) {
+    std::string Value = Args[i];
+    if (Value.empty())
+      report_fatal_error("Empty option string for -recip.");
+    ParseIndividualParams(Value);
+  }
+}
+
+bool TargetRecip::isEnabled(RecipOps Op) const {
+  if (Op == RO_NUM_RECIP_OPS) return false;
+  assert(Enabled[Op] != Uninitialized && "Enabled setting was not initialized");
+  return Enabled[Op];
+}
+
+unsigned TargetRecip::getRefinementSteps(RecipOps Op) const {
+  if (Op == RO_NUM_RECIP_OPS) return 0;
+  assert(RefinementSteps[Op] != Uninitialized &&
+         "Refinement step setting was not initialized");
+  return RefinementSteps[Op];
+}
+
+void TargetRecip::setDefaults(RecipOps Op, bool Enable, unsigned RefSteps) {
+  if (Op == RO_All) {
+    for (int i = 0; i != RO_NUM_RECIP_OPS; ++i) {
+      if (Enabled[i] == Uninitialized)
+        Enabled[i] = Enable;
+      if (RefinementSteps[i] == Uninitialized)
+        RefinementSteps[i] = RefSteps;
+    }
+  } else {
+    if (Enabled[Op] == Uninitialized)
+      Enabled[Op] = Enable;
+    if (RefinementSteps[Op] == Uninitialized)
+      RefinementSteps[Op] = RefSteps;
+  }
+}
+
+bool TargetRecip::operator==(const TargetRecip &Other) const {
+  for (int i = 0; i != RO_NUM_RECIP_OPS; ++i) {
+    if (RefinementSteps[i] != Other.RefinementSteps[i])
+      return false;
+    if (Enabled[i] != Other.Enabled[i])
+      return false;
+  }
+  return true;
+}
+
+TargetRecip::~TargetRecip() {
+}

lib/Target/X86/X86.td

def FeatureCallRegIndirect : SubtargetFeature<"call-reg-indirect",
                                      "CallRegIndirect", "true",
                                      "Call register indirect">;
 def FeatureLEAUsesAG : SubtargetFeature<"lea-uses-ag", "LEAUsesAG", "true",
                                    "LEA instruction needs inputs at AG stage">;
 def FeatureSlowLEA : SubtargetFeature<"slow-lea", "SlowLEA", "true",
                                    "LEA instruction with certain arguments is slow">;
 def FeatureSlowIncDec : SubtargetFeature<"slow-incdec", "SlowIncDec", "true",
                                    "INC and DEC instructions are slower than ADD and SUB">;
-def FeatureUseSqrtEst : SubtargetFeature<"use-sqrt-est", "UseSqrtEst", "true",
-                            "Use RSQRT* to optimize square root calculations">;
-def FeatureUseRecipEst : SubtargetFeature<"use-recip-est", "UseReciprocalEst",
-                          "true", "Use RCP* to optimize division calculations">;
 def FeatureSoftFloat
     : SubtargetFeature<"soft-float", "UseSoftFloat", "true",
                        "Use software floating point features.">;
 
 //===----------------------------------------------------------------------===//
 // X86 processors supported.
 //===----------------------------------------------------------------------===//
 
def : Proc<"btver1",          [FeatureSSSE3, FeatureSSE4A, FeatureCMPXCHG16B,
                                FeatureSlowSHLD]>;
 
 // Jaguar
 def : ProcessorModel<"btver2", BtVer2Model,
                      [FeatureAVX, FeatureSSE4A, FeatureCMPXCHG16B,
                       FeaturePRFCHW, FeatureAES, FeaturePCLMUL,
                       FeatureBMI, FeatureF16C, FeatureMOVBE,
                       FeatureLZCNT, FeaturePOPCNT, FeatureFastUAMem,
-                      FeatureSlowSHLD, FeatureUseSqrtEst, FeatureUseRecipEst]>;
+                      FeatureSlowSHLD]>;
 
 // TODO: We should probably add 'FeatureFastUAMem' to all of the AMD chips.
 
 // Bulldozer
 def : Proc<"bdver1",          [FeatureXOP, FeatureFMA4, FeatureCMPXCHG16B,
                                FeatureAES, FeaturePRFCHW, FeaturePCLMUL,
                                FeatureAVX, FeatureSSE4A, FeatureLZCNT,
                                FeaturePOPCNT, FeatureSlowSHLD]>;

lib/Target/X86/X86ISelLowering.cpp

 STATISTIC(NumTailCalls, "Number of tail calls");
 
 static cl::opt<bool> ExperimentalVectorWideningLegalization(
     "x86-experimental-vector-widening-legalization", cl::init(false),
     cl::desc("Enable an experimental vector type legalization through widening "
              "rather than promotion."),
     cl::Hidden);
 
-static cl::opt<int> ReciprocalEstimateRefinementSteps(
-    "x86-recip-refinement-steps", cl::init(1),
-    cl::desc("Specify the number of Newton-Raphson iterations applied to the "
-             "result of the hardware reciprocal estimate instruction."),
-    cl::NotHidden);
-
 // Forward declarations.
 static SDValue getMOVL(SelectionDAG &DAG, SDLoc dl, EVT VT, SDValue V1,
                        SDValue V2);
 
 X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
                                      const X86Subtarget &STI)
     : TargetLowering(TM), Subtarget(&STI) {
   X86ScalarSSEf64 = Subtarget->hasSSE2();

 }
 
 /// The minimum architected relative accuracy is 2^-12. We need one
 /// Newton-Raphson step to have a good float result (24 bits of precision).
 SDValue X86TargetLowering::getRsqrtEstimate(SDValue Op,
                                             DAGCombinerInfo &DCI,
                                             unsigned &RefinementSteps,
                                             bool &UseOneConstNR) const {
-  // FIXME: We should use instruction latency models to calculate the cost of
-  // each potential sequence, but this is very hard to do reliably because
-  // at least Intel's Core* chips have variable timing based on the number of
-  // significant digits in the divisor and/or sqrt operand.
-  if (!Subtarget->useSqrtEst())
-    return SDValue();
-
   EVT VT = Op.getValueType();
+  RecipOps RecipOp;
 
-  // SSE1 has rsqrtss and rsqrtps.
+  // SSE1 has rsqrtss and rsqrtps. AVX adds a 256-bit variant for rsqrtps.
   // TODO: Add support for AVX512 (v16f32).
   // It is likely not profitable to do this for f64 because a double-precision
   // rsqrt estimate with refinement on x86 prior to FMA requires at least 16
   // instructions: convert to single, rsqrtss, convert back to double, refine
   // (3 steps = at least 13 insts). If an 'rsqrtsd' variant was added to the ISA
   // along with FMA, this could be a throughput win.
-  if ((Subtarget->hasSSE1() && (VT == MVT::f32 || VT == MVT::v4f32)) ||
-      (Subtarget->hasAVX() && VT == MVT::v8f32)) {
-    RefinementSteps = 1;
-    UseOneConstNR = false;
-    return DCI.DAG.getNode(X86ISD::FRSQRT, SDLoc(Op), VT, Op);
-  }
-  return SDValue();
-}
+  if (VT == MVT::f32 && Subtarget->hasSSE1())
+    RecipOp = RO_SqrtF;
+  else if ((VT == MVT::v4f32 && Subtarget->hasSSE1()) ||
+           (VT == MVT::v8f32 && Subtarget->hasAVX()))
+    RecipOp = RO_VecSqrtF;
+  else
+    return SDValue();
+  
+  TargetRecip Recips = DCI.DAG.getTarget().Options.Reciprocals;
+  if (!Recips.isEnabled(RecipOp))
+    return SDValue();
+  
+  RefinementSteps = Recips.getRefinementSteps(RecipOp);
 
 /// The minimum architected relative accuracy is 2^-12. We need one
 /// Newton-Raphson step to have a good float result (24 bits of precision).
 SDValue X86TargetLowering::getRecipEstimate(SDValue Op,
                                             DAGCombinerInfo &DCI,
                                             unsigned &RefinementSteps) const {
-  // FIXME: We should use instruction latency models to calculate the cost of
-  // each potential sequence, but this is very hard to do reliably because
-  // at least Intel's Core* chips have variable timing based on the number of
-  // significant digits in the divisor.
-  if (!Subtarget->useReciprocalEst())
-    return SDValue();
-
   EVT VT = Op.getValueType();
+  RecipOps RecipOp;
   
   // SSE1 has rcpss and rcpps. AVX adds a 256-bit variant for rcpps.
   // TODO: Add support for AVX512 (v16f32).
   // It is likely not profitable to do this for f64 because a double-precision
   // reciprocal estimate with refinement on x86 prior to FMA requires
   // 15 instructions: convert to single, rcpss, convert back to double, refine
   // (3 steps = 12 insts). If an 'rcpsd' variant was added to the ISA
   // along with FMA, this could be a throughput win.
-  if ((Subtarget->hasSSE1() && (VT == MVT::f32 || VT == MVT::v4f32)) ||
-      (Subtarget->hasAVX() && VT == MVT::v8f32)) {
-    RefinementSteps = ReciprocalEstimateRefinementSteps;
-    return DCI.DAG.getNode(X86ISD::FRCP, SDLoc(Op), VT, Op);
-  }
+  if (VT == MVT::f32 && Subtarget->hasSSE1())
+    RecipOp = RO_DivF;
+  else if ((VT == MVT::v4f32 && Subtarget->hasSSE1()) ||
+           (VT == MVT::v8f32 && Subtarget->hasAVX()))
+    RecipOp = RO_VecDivF;
+  else
     return SDValue();
+  
+  TargetRecip Recips = DCI.DAG.getTarget().Options.Reciprocals;
+  if (!Recips.isEnabled(RecipOp))
+    return SDValue();
+
+  RefinementSteps = Recips.getRefinementSteps(RecipOp);
+  return DCI.DAG.getNode(X86ISD::FRCP, SDLoc(Op), VT, Op);
 }
 
 /// If we have at least two divisions that use the same divisor, convert to
 /// multplication by a reciprocal. This may need to be adjusted for a given
 /// CPU if a division's cost is not at least twice the cost of a multiplication.
 /// This is because we still need one division to calculate the reciprocal and
 /// then we need two multiplies by that reciprocal as replacements for the
 /// original divisions.

lib/Target/X86/X86Subtarget.h

protected:
   bool LEAUsesAG;
 
   /// True if the LEA instruction with certain arguments is slow
   bool SlowLEA;
 
   /// True if INC and DEC instructions are slow when writing to flags
   bool SlowIncDec;
 
-  /// Use the RSQRT* instructions to optimize square root calculations.
-  /// For this to be profitable, the cost of FSQRT and FDIV must be
-  /// substantially higher than normal FP ops like FADD and FMUL.
-  bool UseSqrtEst;
-
-  /// Use the RCP* instructions to optimize FP division calculations.
-  /// For this to be profitable, the cost of FDIV must be
-  /// substantially higher than normal FP ops like FADD and FMUL.
-  bool UseReciprocalEst;
-
   /// Processor has AVX-512 PreFetch Instructions
   bool HasPFI;
 
   /// Processor has AVX-512 Exponential and Reciprocal Instructions
   bool HasERI;
 
   /// Processor has AVX-512 Conflict Detection Instructions
   bool HasCDI;
public:
   bool useLeaForSP() const { return UseLeaForSP; }
   bool hasSlowDivide32() const { return HasSlowDivide32; }
   bool hasSlowDivide64() const { return HasSlowDivide64; }
   bool padShortFunctions() const { return PadShortFunctions; }
   bool callRegIndirect() const { return CallRegIndirect; }
   bool LEAusesAG() const { return LEAUsesAG; }
   bool slowLEA() const { return SlowLEA; }
   bool slowIncDec() const { return SlowIncDec; }
-  bool useSqrtEst() const { return UseSqrtEst; }
-  bool useReciprocalEst() const { return UseReciprocalEst; }
   bool hasCDI() const { return HasCDI; }
   bool hasPFI() const { return HasPFI; }
   bool hasERI() const { return HasERI; }
   bool hasDQI() const { return HasDQI; }
   bool hasBWI() const { return HasBWI; }
   bool hasVLX() const { return HasVLX; }
 
   bool isAtom() const { return X86ProcFamily == IntelAtom; }

lib/Target/X86/X86Subtarget.cpp

void X86Subtarget::initializeEnvironment() {
   UseLeaForSP = false;
   HasSlowDivide32 = false;
   HasSlowDivide64 = false;
   PadShortFunctions = false;
   CallRegIndirect = false;
   LEAUsesAG = false;
   SlowLEA = false;
   SlowIncDec = false;
-  UseSqrtEst = false;
-  UseReciprocalEst = false;
   stackAlignment = 4;
   // FIXME: this is a known good value for Yonah. How about others?
   MaxInlineSizeThreshold = 128;
   UseSoftFloat = false;
 }
 
 X86Subtarget &X86Subtarget::initializeSubtargetDependencies(StringRef CPU,
                                                             StringRef FS) {

lib/Target/X86/X86TargetMachine.cpp

X86TargetMachine::X86TargetMachine(const Target &T, StringRef TT, StringRef CPU,
 
   // Windows stack unwinder gets confused when execution flow "falls through"
   // after a call to 'noreturn' function.
   // To prevent that, we emit a trap for 'unreachable' IR instructions.
   // (which on X86, happens to be the 'ud2' instruction)
   if (Subtarget.isTargetWin64())
     this->Options.TrapUnreachable = true;
 
+  this->Options.Reciprocals.setDefaults(RO_All, false, 1);
   initAsmInfo();
 }
 
 X86TargetMachine::~X86TargetMachine() {}
 
 const X86Subtarget *
 X86TargetMachine::getSubtargetImpl(const Function &F) const {
   Attribute CPUAttr = F.getFnAttribute("target-cpu");

test/CodeGen/X86/recip-fastmath.ll

 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=sse2 | FileCheck %s
-; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx,use-recip-est | FileCheck %s --check-prefix=RECIP
-; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx,use-recip-est -x86-recip-refinement-steps=2 | FileCheck %s --check-prefix=REFINE
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx -recip=divf,vec-divf | FileCheck %s --check-prefix=RECIP
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx -recip=divf:2,vec-divf:2 | FileCheck %s --check-prefix=REFINE
 
 ; If the target's divss/divps instructions are substantially
 ; slower than rcpss/rcpps with a Newton-Raphson refinement,
 ; we should generate the estimate sequence.
 
 ; See PR21385 ( http://llvm.org/bugs/show_bug.cgi?id=21385 )
 ; for details about the accuracy, speed, and implementation
 ; differences of x86 reciprocal estimates.

test/CodeGen/X86/sqrt-fastmath.ll

 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=sse2 | FileCheck %s
-; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx,use-sqrt-est | FileCheck %s --check-prefix=ESTIMATE
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=avx -recip=sqrtf,vec-sqrtf | FileCheck %s --check-prefix=ESTIMATE
 
 declare double @__sqrt_finite(double) #0
 declare float @__sqrtf_finite(float) #0
 declare x86_fp80 @__sqrtl_finite(x86_fp80) #0
 declare float @llvm.sqrt.f32(float) #0
 declare <4 x float> @llvm.sqrt.v4f32(<4 x float>) #0
 declare <8 x float> @llvm.sqrt.v8f32(<8 x float>) #0