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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D149715#4381350 <https://reviews.llvm.org/D149715#4381350>, @sepavloff wrote:

> There is no code for WidenVecRes_FREXP and WidenVecOp_FREXP?

I also don't know of a use case for them, so no way to test it



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2558
+
+  SDValue ExpMaskScaled =
+      DAG.getNode(ISD::AND, dl, AsIntVT, ScaledAsInt, ExpMask);
----------------
sepavloff wrote:
> 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.
This passed when I forced passing this through opencl conformance on amdgpu. I don't really want to touch this anymore, especially since the combiner should take care of these types of folds


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

https://reviews.llvm.org/D149715