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

[ChenZheng via Phabricator via llvm-commits]

shchenz added a comment.

I think it's better to add a denormal test case



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1437
+  case PPCISD::FTSQRT:
+    return "PPCISD::FTSQRT";
   case PPCISD::STFIWX:          return "PPCISD::STFIWX";
----------------
Better to follow legacy formatting or reformat all the sentences here?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:12386
+
+  // TODO - add support for v2f64/v4f32
+  SDLoc DL(Op);
----------------
Move the comments above line 12383?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:12389
+  // The output register of FTSQRT is CR field.
+  SDValue FTSQRT = DAG.getNode(PPCISD::FTSQRT, DL, MVT::i32, Op);
+  // ftsqrt BF,FRB
----------------
Any specific reason for i32 here? I guess here i8 would be enough


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:12398
+  // Otherwise fe_flag is set to 0.
+  SDValue SRIdxVal = DAG.getTargetConstant(PPC::sub_eq, DL, MVT::i32);
+  return SDValue(DAG.getMachineNode(TargetOpcode::EXTRACT_SUBREG, DL, MVT::i1,
----------------
Target independent code checks denormal input, `ftsqrt` denormal input checking result is in `fg_flag`, your comments seems like related to fe_flag, does this matters?


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:624
+                          "xstsqrtdp $crD, $XB", IIC_FPCompare,
+                          [(set i32:$crD, (PPCftsqrt f64:$XB))]>;
 
----------------
Same as above, i8 should be enough for crD? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80706