[PATCH] D140381: [PowerPC] Use Power9 test data class instruction to lower IS_FPCLASS

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11010
+                                SelectionDAG &DAG,
+                                const PPCSubtarget &Subtarget) {
+  assert(Mask <= fcAllFlags && "Invalid fp_class flags!");
----------------
Should we check `isKnownNeverNaN()` or `isKnownNeverZeroFloat()` first before we really compute the class? (we may need to add `isKnownNeverInfinity()` and  `isKnownNeverSubnormal()`). With these functions, we can handle some compile-time class. For example, the `Op` is a fp constant.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11066
+      Sign = DAG.getNOT(Dl, Sign, MVT::i1);
+    return DAG.getNode(ISD::OR, Dl, MVT::i1, Rest,
+                       DAG.getNode(ISD::AND, Dl, MVT::i1, Sign, Normal));
----------------
Do we need the `OR` if `Mask == fcPosNormal || Mask == fcNegNormal` ?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11103
+    SDValue NanRes = DAG.getSetCC(
+        Dl, MVT::i1,
+        DAG.getNode(ISD::AND, Dl, MVT::i32, HighWord,
----------------
Will we generate an illegal type if `useCRBits()` is set to false?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11107
+        DAG.getConstant(0, Dl, MVT::i32), IsQuiet ? ISD::SETNE : ISD::SETEQ);
+    return DAG.getNode(ISD::OR, Dl, MVT::i1, Rest,
+                       DAG.getNode(ISD::AND, Dl, MVT::i1, NanCheck, NanRes));
----------------
Same as above, we may not need the `OR` is we only test SNAN or QNAN?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140381