[llvm-commits] [PATCH] Compress Repeated Byte Output

[David A. Greene]

Chris Lattner <clattner at apple.com> writes:

> Looks good with a few changes:
>
> +static bool isRepeatedByteSequence(const ConstantInt *CI, TargetMachine &TM) {
>
> This needs a doxygen comment, also please change it to be something like:

Ok.

> static int isRepeatedByteSequence(const Value *V, TargetMachine &TM) {
>
> and have it return 0-255 for success and -1 for failure.  It should
> handle the non-ConstantInt case as well.  You could even generalize
> this predicate to handle structs and subarrays if you feel ambitious
> (but as a follow-on patch).

Are you saying the return value should be the value of the repeated
byte?

> +  if (CI->isZero() ||
> +      CI->isAllOnesValue() ||
> +      TM.getTargetData()->getTypeAllocSize(CI->getType()) == 1)
> +    return true;
> +
> +  unsigned Bytes = TM.getTargetData()->getTypeAllocSize(CI->getType());
>
> Please don't call getTypeAllocSize twice, use:

Good catch.

> +  if (CI->isZero() || CI->isAllOnesValue())
>     return true;
>
> +  unsigned Bytes = TM.getTargetData()->getTypeAllocSize(CI->getType());
>   if (Bytes == 1) return true;
> ...
>
>
> +  uint64_t Value = CI->getZExtValue();
>
> This will abort for int128_t, please check that the bitwidth of CI is <= 64 bits and a multiple of 8 bits.

Ah, ok.

                             -Dave