[PATCH] D51751: Merge clang's isRepeatedBytePattern with LLVM's isBytewiseValue

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: test/Transforms/MemCpyOpt/memcpy-to-memset.ll:54
+
+ at i1x16 = internal constant <16 x i1> <i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0, i1 0>, align 4
+define void @test_i1x16() nounwind {
----------------
jfb wrote:
> jfb wrote:
> > efriedma wrote:
> > > You're not really testing any interesting codepaths with a constant zero?  We just hit the "isNullValue()" early exit.
> > It's the only interesting codepath for `i1` though. Unless you're saying that we should add handling for `i1` in `isBytewiseValue`?
> To be clear, this is where we return `nullptr` for all other `i1`:
> 
> ```
>   // We can handle constant integers that are multiple of 8 bits.
>   if (ConstantInt *CI = dyn_cast<ConstantInt>(C)) {
>     if (CI->getBitWidth() % 8 == 0) {
>       assert(CI->getBitWidth() > 8 && "8 bits should be handled above!");
>       if (!CI->getValue().isSplat(8))
>         return nullptr;
>       return ConstantInt::get(Ctx, CI->getValue().trunc(8));
>     }
>   }
> ```
Well, an improved isBytewiseValue could theoretically handle it... but I guess it's probably not worth bothering, at least for now.  But better to have some test coverage so we it's clear what isn't handled.


Repository:
  rL LLVM

https://reviews.llvm.org/D51751