[PATCH] D19426: [AArch64] Use the reciprocal estimation machinery

[Evgeny Astigeevich via llvm-commits]

eastig added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64.td:67
@@ -61,1 +66,3 @@
+   "Enable the approximation of floating-point division", [FeatureNEON]>;
+
 //===----------------------------------------------------------------------===//
----------------
I think it's better to use names something like:
FeatureReciprocalSqrtEstimate
FeatureReciprocalEstimate

They don't directly calculate DIV and SQRT. They calculate aproximate 1/DIV and 1/SQRT.

================
Comment at: llvm/lib/Target/AArch64/AArch64.td:142
@@ -135,1 +141,3 @@
+                                    FeaturePerfMon,
+                                    FeatureApproximateSqrt]>;
 
----------------
Why is FeatureApproximateDiv not on the list of the features?

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4663
@@ -4623,1 +4662,3 @@
+}
+
 //===----------------------------------------------------------------------===//
----------------
Most of the code of the functions is the same. Only differences are RecipOp string and Opcodes. I would suggest to create a template function or a function with common code. Also documenting parameters will be good. Am I correct they will be used for step versions in the future?

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:178
@@ -153,1 +177,3 @@
+    ("vec-divd", Subtarget.hasApproximateDiv(), ExtraStepsD);
+
   initAsmInfo();
----------------
I suggest to put this code into a separate function(s): initReciprocals. This will help not to make mess if in the future more initialization is added.


Repository:
  rL LLVM

http://reviews.llvm.org/D19426