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

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang marked 8 inline comments as done.
steven.zhang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1437
+  case PPCISD::FTSQRT:
+    return "PPCISD::FTSQRT";
   case PPCISD::STFIWX:          return "PPCISD::STFIWX";
----------------
shchenz wrote:
> Better to follow legacy formatting or reformat all the sentences here?
It is format by clang-format. Maybe, we can commit a NFC patch to format all the statements here.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:12386
+
+  // TODO - add support for v2f64/v4f32
+  SDLoc DL(Op);
----------------
shchenz wrote:
> Move the comments above line 12383?
Ok, I will do it with later change with this patch.


================
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
----------------
shchenz wrote:
> Any specific reason for i32 here? I guess here i8 would be enough
It is because the type of the CRRC must be i32.
```
def CRRC : RegisterClass<"PPC", [i32], 32,
  (add CR0, CR1, CR5, CR6,
       CR7, CR2, CR3, CR4)> {
  let AltOrders = [(sub CRRC, CR2, CR3, CR4)];
  let AltOrderSelect = [{
    return MF.getSubtarget<PPCSubtarget>().isELFv2ABI() &&
           MF.getInfo<PPCFunctionInfo>()->isNonVolatileCRDisabled();
  }];
}
```



================
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,
----------------
shchenz wrote:
> 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?
Target independent code didn't assume the content of the check and the target is free to do the kind of check. The ISA contains a program note:
```
ftdiv and ftsqrt are provided to accelerate software
emulation of divide and square root operations, by
performing the requisite special case checking.
Software needs only a single branch, on FE=1 (in
CR[BF]), to a special case handler. FG and FL may
provide further acceleration opportunities.
```
So, I select the FE for the special case handle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80706