[PATCH] [x86] make reciprocal estimate code generation more flexible

[hfinkel at anl.gov]

================
Comment at: include/llvm/Target/TargetOptions.h:240
@@ -237,1 +239,3 @@
 
+    /// This class encapsulates options for reciprocal estimate code generation.
+    TargetRecip Reciprocals;
----------------
reciprocal estimate -> reciprocal-estimate

================
Comment at: include/llvm/Target/TargetRecip.h:34
@@ +33,3 @@
+  VecSqrtD,     // square root, double, vector
+  INVALID
+};
----------------
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).


================
Comment at: include/llvm/Target/TargetRecip.h:34
@@ +33,3 @@
+  VecSqrtD,     // square root, double, vector
+  INVALID
+};
----------------
hfinkel wrote:
> 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).
> 
If you're using INVALID for the "number of", please name it NUM_RECIP_OPS (or similar).


================
Comment at: include/llvm/Target/TargetRecip.h:48
@@ +47,3 @@
+  /// Set whether a particular reciprocal operation is enabled and how many
+  /// refinement steps are needed when using it. Use the INVALID operation
+  /// to set enablement and refinement steps for all operations.
----------------
Let's not use INVALID for "all". You can add an All to the enum (it can even have the same value as INVALID).

================
Comment at: lib/Target/TargetRecip.cpp:58
@@ +57,3 @@
+  // step parameter.
+  assert(RefStepString.length() == 1 && "Invalid refinement step for -recip.");
+  char RefStepChar = RefStepString[0];
----------------
These strings are user-provided, we can't assert on invalid inputs.

================
Comment at: lib/Target/TargetRecip.cpp:148
@@ +147,3 @@
+  unsigned NumStrings = llvm::array_lengthof(ArgStrings);
+  assert(NumArgs <= NumStrings && "Too many options for -recip.");
+
----------------
Hrmm, there could be duplicates. Just parse them in order (users may provide duplicates).

http://reviews.llvm.org/D8982

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/