[PATCH] D22238: Reapply "InstCombine: Reduce trunc (shl x, K) width."

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:536-537
@@ -535,4 +535,4 @@
 
   // Transform "trunc (and X, cst)" -> "and (trunc X), cst" so long as the dest
-  // type isn't non-native.
+  // type is native.
   if (Src->hasOneUse() && isa<IntegerType>(SrcTy) &&
----------------
This comment seems duplicated.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:550
@@ +549,3 @@
+    // Transform "trunc (shl X, cst)" -> "shl (trunc X), cst" so long as the
+    // dest type isn't non-native and cst < dest size.
+    if (match(Src, m_Shl(m_Value(A), m_ConstantInt(Cst))) &&
----------------
Can we replace "isn't non-native" with "is native"?

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:551
@@ +550,3 @@
+    // dest type isn't non-native and cst < dest size.
+    if (match(Src, m_Shl(m_Value(A), m_ConstantInt(Cst))) &&
+        !match(A, m_Shr(m_Value(), m_Constant()))) {
----------------
Consider using `m_APInt` here, it handles vector splats.

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:555
@@ +554,3 @@
+      // FoldShiftByConstant and is the extend in reg pattern.
+      const unsigned DestSize = DestTy->getPrimitiveSizeInBits();
+      if (Cst->getValue().ult(DestSize)) {
----------------
I think it'd be nice to use getScalarSizeInBits so that the code is more vector ready.

================
Comment at: test/Transforms/InstCombine/2011-05-28-swapmulsub.ll:36
@@ -36,1 +35,3 @@
+; CHECK-NEXT: trunc i32
+; CHECK-NEXT: shl i16
   store i32 %mul, i32* %a, align 4
----------------
What happens with this i16?


https://reviews.llvm.org/D22238