[PATCH] D22975: Add new nodes for computing the Newton series

[Nikolai Bozhenov via llvm-commits]

n.bozhenov added inline comments.

================
Comment at: llvm/include/llvm/CodeGen/ISDOpcodes.h:546
@@ -540,1 +545,3 @@
+    FRSQRTI,
+
     /// LOAD and STORE have token chains as their first operand, then the same
----------------
I don't quite understand why do you need to introduce new ARM specific nodes into the ISD namespace. As an alternative, you could ask the target to directly build ARMISD::FRSQRTI instructions from buildSqrtNRNative function.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14695
@@ -14655,1 +14694,3 @@
+              ? buildSqrtNROneConst(Op, Est, Iterations, Flags, Reciprocal)
+              : buildSqrtNRTwoConst(Op, Est, Iterations, Flags, Reciprocal);
     }
----------------
A more consistent way to choose a refinement method would be to replace the boolean UseOneConstNR value with some enum, let the target set this enum and choose the refinement method based on the enum value.

Yet another option would be to return an already refined value from ARMTargetLowering::getRsqrtEstimate and set Iterations to 0. In this case we wouldn't need the buildSqrtNRNative function.



Repository:
  rL LLVM

https://reviews.llvm.org/D22975