[PATCH] D124590: [InstCombine] add casts from splat-a-bit pattern if necessary

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1595
   // sext (ashr (trunc iN X to iM), M-1) to iN --> ashr (shl X, N-M), N-1
-  // TODO: If the dest type is different, use a cast (adjust use check).
+  // If the dest type is different, use a cast (adjust use check).
   if (match(Src, m_OneUse(m_AShr(m_Trunc(m_Value(X)),
----------------
The TODO says we need to adjust (add) a use check, but this patch did not do that.

Please add a test like this to check that we have the correct behavior:

```
declare void @use(i8)
define i16 @smear_set_bit_different_dest_type_extra_use(i32 %x) {
  %t = trunc i32 %x to i8
  call void @use(i8 %t)
  %a = ashr i8 %t, 7
  %s = sext i8 %a to i16
  ret i16 %s
}

```


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1606-1608
+      Constant *ShlAmtC = ConstantInt::get(XTy, XBitSize - SrcBitSize);
+      Constant *AshrAmtC = ConstantInt::get(XTy, XBitSize - 1);
+      Value *Shl = Builder.CreateShl(X, ShlAmtC);
----------------
These 3 values are common in both cases - I think it's better to not repeat the code on both sides of the if/else.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:1610
+      Value *Ashr = Builder.CreateAShr(Shl, AshrAmtC);
+      return CastInst::CreateIntegerCast(Ashr, DestTy, /* isSigned */ true);
+    }
----------------
We need another test to verify what happens when DestTy is wider than XTy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124590