[PATCH] D99469: GlobalISel: Restrict narrow scalar for fptoui/fptosi results

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

The code change looks fine to me. The added tests aren't actually testing any behaviour that has changed, are they? Is there any reasonable way to add tests for the unable-to-legalize case?



================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4714
+  LLT SrcTy = MRI.getType(Src);
+  LLT NarrowDstTy = NarrowTy;
+
----------------
There's no need to introduce NarrowDstTy, just use NarrowTy throughout.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4719
+  // half to at least 16-bits, so just handle the one case.
+  if (SrcTy.getScalarType() == LLT::scalar(16) &&
+      NarrowDstTy.getScalarSizeInBits() >= (IsSigned ? 17 : 16)) {
----------------
Could invert the condition and early-out here but it doesn't really matter.


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

https://reviews.llvm.org/D99469