[PATCH] D54749: Saturating float to int casts.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> I presume it's fine to only fix the ones that don't conform to some existing unconventional code layout?

Yes.

> By expand into vector ops, you mean to avoid breaking up the vector operation into scalar operations and then legalizing those, and instead expanding the vector operation directly to the corresponding operations? What would be the criteria for doing this rather than splitting it up first?

Yes.  Probably sufficient to guard this with a check that the vector operations in question are legal.

> It was originally a type operand similar to SIGN_EXTEND_INREG, but the suggestion was to replace it with a constant operand instead to make things simpler. The only thing we care about is the saturation bit width.

Not a big deal either way.

> It might have been fine to make it a scalar type operand even for vector types, though.

This is how SIGN_EXTEND_INREG works, I think?



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2683
+    if (TLI.isOperationLegalOrCustom(Opcode, NewOutTy))
+      break;
+  }
----------------
ebevhan wrote:
> efriedma wrote:
> > This can't be the only place that uses this loop; can it be refactored?
> There are similar loops in some of the other PromoteLegal* functions. The loops in those functions could probably be refactored - they look compatible - but this loop isn't quite the same since the signed nodes can't be used to implement the unsigned ones.
Okay.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7987
+    SrcVT = Src.getValueType();
+  }
+
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54749