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

[James Molloy via llvm-commits]

jmolloy added a comment.

Hi Evandro,

This looks much nicer, thanks! Just a couple of readability issues left from my side.

Cheers,

James


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4636
@@ +4635,3 @@
+  std::string RecipOp;
+  RecipOp = Opcode == AArch64ISD::FRECPE? "div": "sqrt";
+  RecipOp = (VT.isVector()? "vec-": "") + RecipOp;
----------------
For readability, please could you put the test here in brackets and have a space before the '?' of the ternary? Same below:

  std::string RecipOp = (AArch64ISD::FRECPE) ? "div" : "sqrt";

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:150
@@ +149,3 @@
+
+  TM.Options.Reciprocals.setDefaults("sqrtf", ST.isExynosM1(), ExtraStepsF);
+  TM.Options.Reciprocals.setDefaults("sqrtd", ST.isExynosM1(), ExtraStepsD);
----------------
Please can you extract the heuristic computation from the call here, to make it explicit that's what it is?

Something like:

    // FIXME: Only enable SQRT reciprocals for M1 for the moment.
    bool UseRSQRTE = ST.isExynosM1();
    TM.Options.Reciprocals.setDefaults("sqrtf", UseRSQRTE, ExtraStepsF);


Repository:
  rL LLVM

http://reviews.llvm.org/D19426