[PATCH] D83671: GlobalISel: Implement widenScalar for saturating add/sub

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1671
+
+  auto Result = IsSigned ? MIRBuilder.buildAShr(WideTy, WideInst, ShiftK)
+                         : MIRBuilder.buildLShr(WideTy, WideInst, ShiftK);
----------------
arsenm wrote:
> foad wrote:
> > arsenm wrote:
> > > foad wrote:
> > > > There's no need to make this conditional on IsSigned if we're just about to truncate anyway.
> > > How else would I know whether to prefer ashr or lshr?
> > Why would it make any difference? Can't you just pick one arbitrarily?
> First I don't know whether the target has a reason to prefer one over the other. I also would assume using the right shift to preserve the expected number of sign bits for the signed result would be more useful for downstream simplifications once the trunc is folded out
OK, I won't object if you want to leave it like that. The other comment about anyext still stands.


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

https://reviews.llvm.org/D83671