[PATCH] D149715: IR: Add llvm.frexp intrinsic

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2508
+
+  SDValue SmallestNormalizedMask = DAG.getConstant(
+      APFloat::getSmallestNormalized(FltSem, false).bitcastToAPInt(), dl,
----------------
Thos variable is not a mask, it is just a value.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2543
+
+  SDValue AddBiggestNegNormal =
+      DAG.getNode(ISD::ADD, dl, AsIntVT, Abs, NegSmallestNormalizedInt);
----------------
Look like the name is wrong. It states about biggest normal, but actually the smallest is added.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2545
+      DAG.getNode(ISD::ADD, dl, AsIntVT, Abs, NegSmallestNormalizedInt);
+  SDValue NonFiniteOrZero = DAG.getSetCC(dl, SetCCVT, AddBiggestNegNormal,
+                                         NegSmallestNormalizedInt, ISD::SETULE);
----------------
Is this correct?

The check is `x + SNN <= SNN` in unsigned wrapping math. It holds if `x` represents zero or denormal value, so essentially it checks for zero or denormal, no?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2558
+
+  SDValue ExpMaskScaled =
+      DAG.getNode(ISD::AND, dl, AsIntVT, ScaledAsInt, ExpMask);
----------------
This value is used only `SELECT`, which is then shifted right. Can it be dropped?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:5202
+        DAG.getNode(ISD::FP_ROUND, dl, OVT, Tmp2,
+                    DAG.getIntPtrConstant(0, dl, /*isTarget=*/true)));
+
----------------
Can we use 1 for the second operand? The value produced by FFREXP should not add extra bits to mantissa.


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

https://reviews.llvm.org/D149715