[PATCH] D68651: [InstCombine] Signed saturation patterns

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2032-2035
+  // FIXME: This isn't quite right for vectors, but using the scalar type is a
+  // good first approximation for what should be done there.
+  if (!shouldChangeType(Ty->getScalarType()->getIntegerBitWidth(), NewBitWidth))
+    return nullptr;
----------------
lebedev.ri wrote:
> dmgreen wrote:
> > lebedev.ri wrote:
> > > Can you tell which test is not folded because of this check?
> > > 
> > Ah. That was meant to be sadd_sat31, but then I re-added back in some extra checks. I'll change it to a sadd_sat4 which should test this.
> > 
> > I presume it's a good idea to prevent the creation i4's?
> Eh, good point.
> Not sure honestly, i don't remember ever seeing that check in any of the transforms recently-ish added.
@spatel @nikic thoughts? I'm almost thinking this should not be here.


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

https://reviews.llvm.org/D68651