[PATCH] D54749: Saturating float to int casts.

[Bevin Hansson via Phabricator via llvm-commits]

ebevhan added a comment.

In D54749#2242802 <https://reviews.llvm.org/D54749#2242802>, @efriedma wrote:

>> At least for the IEEE formats, you should be able to just destructure the bit-pattern of the float, right? Normalize and do some extends and shifts.
>
> If we're going down that path, maybe simpler to just mess with the float scaling factor.  If a float is close to the largest finite float, in IEEE formats, the fractional part must be zero.  So we can do the float-to-int conversion on the unscaled float, and scale the resulting integer.  Probably faster than trying to explicitly destructure a float.

I'm not sure how this would work. The float does not have to be close to its finite limit in order to contain a valid and representable fixed-point value.

I'm trying to determine if the only criteria for not being able to use a fmul+fptoint-pattern for fixed-point conversion is whether or not the fixed-point scaling factor fits in the exponent. It should be possible to decompose the operation into a bitcast+mask+some shifting in the case where we can't use fmul+fptoint.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1361
+      TLI.isOperationLegal(ISD::FMINNUM, SrcVT))
+    Results.push_back(TLI.expandFP_TO_INT_SAT(Node, DAG));
+}
----------------
I'm unsure if I've done the right thing here. We don't even get to this function for any of the test cases.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7987
+    SrcVT = Src.getValueType();
+  }
+
----------------
efriedma wrote:
> ebevhan wrote:
> > efriedma wrote:
> > > ebevhan wrote:
> > > > I also added this to solve issues in expanding f16 FP_TO_XINT nodes to libcalls. Libcalls with a source of f16 don't seem to exist, so we can't emit such nodes if we will end up making libcalls of them.
> > > > 
> > > > Maybe this should check operation and type legality, though.
> > > In theory, legalization should ensure we generate appropriate libcalls.
> > > 
> > > Maybe that doesn't actually happen at the moment, though.
> > There's some conditions in the libcall emission which do cause promotion of the source before emission, but it doesn't happen in every case (even though no libcalls from half->int seem to exist in the first place).
> Maybe we have the relevant code in type legalization, but not LegalizeDAG, or something like that?
I think the spot that I had issues with was during integer result expansion, actually. ExpandIntRes_FP_TO_SINT.

If the integer result type must be expanded, but the source type is f16 (and legal), it will try to emit a libcall which does not exist. It was probably f16 -> i128 in my case.

There are cases for promotion and softening in the function, but those only take effect if the float operand must be promoted or softened, which it doesn't have to be if the type is legal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54749