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

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

There is no code for WidenVecRes_FREXP and WidenVecOp_FREXP?



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2558
+
+  SDValue ExpMaskScaled =
+      DAG.getNode(ISD::AND, dl, AsIntVT, ScaledAsInt, ExpMask);
----------------
arsenm wrote:
> sepavloff wrote:
> > This value is used only `SELECT`, which is then shifted right. Can it be dropped?
> Not sure I follow, you would have to shift the other operand of the select too. 
> 
> I got here by running the C implementation through the optimizer and translating from IR up to the DAG nodes. I'm trusting the IR's canonicalization decisions. 
The value `ExpMaskScaled` is used only in SELECT:
```
  SDValue ScaledValue =
      DAG.getNode(ISD::SELECT, dl, AsIntVT, IsDenormal, ExpMaskScaled, Abs);
```
which is used only in the right shift:
```
  SDValue ShiftedExp =
      DAG.getNode(ISD::SRL, dl, AsIntVT, ScaledValue, ExponentShiftAmt);
```
It shifts out the bits that were cleared by AND operation, so this operation looks useless and may be removed.

> I got here by running the C implementation through the optimizer and translating from IR up to the DAG nodes.

Probably this implementation may be used as is if its correctness is validated in some other way, maybe by runtime tests. Cleanup and optimization may be made later on.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:337
+
+  assert(ResNo == 0 && "todo");
+  EVT OpVT = Op.getValueType();
----------------
Is this code unreachable?


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

https://reviews.llvm.org/D149715