[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]>;
+
 //===----------------------------------------------------------------------===//
----------------
evandro wrote:
> eastig wrote:
> > 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.
> Do you mean to replace HasApproximateSqrt with FeatureReciprocalSqrtEstimate and so on?
Yes.
When I first read the names of these definitions I had initial understanding based on their names. Later I saw instructions and from their documentation I found what they do. It didn't match to my initial understanding. I think when someone who is not familiar with these instructions starts looking at the code he or she will have the similar issue.

================
Comment at: llvm/lib/Target/AArch64/AArch64.td:142
@@ -135,1 +141,3 @@
+                                    FeaturePerfMon,
+                                    FeatureApproximateSqrt]>;
 
----------------
evandro wrote:
> eastig wrote:
> > Why is FeatureApproximateDiv not on the list of the features?
> Because Exynos M1 doesn't benefit from it.  It might be beneficial in other cores, when their respective maintainers should be better equipped than me to decide whether to add such features or not.
Sorry for a naive question.
Do we need to have these feature on the list? I thought they are part ARMv8-A NEON.


Repository:
  rL LLVM

http://reviews.llvm.org/D19426