[PATCH] D80706: [DAGCombine] Add hook to allow target specific test for sqrt input

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:4274
+  /// result should be used as the condition operand for a select or branch.
+  virtual SDValue getSqrtInputTest(SDValue Operand, SelectionDAG &DAG,
+                                   const DenormalMode &Mode) const {
----------------
qiucf wrote:
> `testSqrtEstimate`?
Hmm, I still prefer the getXXX as we have getSqrtEstimate and getRecipEstimate likewise routine.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:21959
+        // Try the target specific test first.
+        SDValue Test = TLI.getSqrtInputTest(Op, DAG, DenormMode);
+        if (!Test) {
----------------
qiucf wrote:
> Will it be better if put logic below into base `getSqrtInputTest` implementation?
I think both are ok. The good things for this is to have the target specific test inside getSqrtInputTest() and return SDVaue() if didn't have. We are making the default implementation non-override which makes sense in fact till now.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:12684-12691
+  // ftsqrt BF,FRB
+  // Let e_b be the unbiased exponent of the double-precision
+  // floating-point operand in register FRB.
+  // fe_flag is set to 1 if either of the following conditions occurs.
+  //   - The double-precision floating-point operand in register FRB is a zero,
+  //     a NaN, or an infinity, or a negative value.
+  //   - e_b is less than or equal to -970.
----------------
qiucf wrote:
> 
Thank you for this. I will update it.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrFormats.td:643
   let FRA = 0;
+  let Pattern =  pattern;
 }
----------------
qiucf wrote:
> Typo: extra space
Good catch. Update it when I commit it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80706/new/

https://reviews.llvm.org/D80706