[PATCH] D27466: [SLP] Fix sign-extends for type-shrinking

[Gerolf Hoflehner via Phabricator via llvm-commits]

Gerolf requested changes to this revision.
Gerolf added a reviewer: Gerolf.
Gerolf added a comment.
This revision now requires changes to proceed.

Michael, could you also lend this your expert eye? Do you agree with the extra memory (pair vs vector) and compile-time (look at the loop tree) investment for this fix? Shortening the types may not be worth it at least on some architectures.

Thanks
Gerolf



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1939
       VecTy = VectorType::get(MinTy, BundleWidth);
-      ExtractCost += TTI->getExtractWithExtendCost(
-          Instruction::SExt, EU.Scalar->getType(), VecTy, EU.Lane);
+      ExtractCost += TTI->getExtractWithExtendCost(ExTy, EU.Scalar->getType(),
+                                                   VecTy, EU.Lane);
----------------
I would expect the TTI function to make use of the extra information of the sign bit, but can't see that code. Or is this intentional?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3521
+  // value is known to be zero.
+  auto isSignBitKnownZero = [&](Value *V) -> bool {
+    bool KnownZero = false;
----------------
This looks convoluted. You define a lambda and then use the inverted value. I think what you want i hasSignBitSet[] which returns is KnownOne. Then you use this as IsSigned = hasSignBitSet(). I think in your code you mixup concepts and   intended use.


https://reviews.llvm.org/D27466