[PATCH] D40339: Use getStoreSize() in various places instead of BitSize >> 3

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In https://reviews.llvm.org/D40339#937625, @jonpa wrote:

> Thanks for review. r319173.
>
> BTW,
>
> What about all the cases using Type* , like 'ByteSize = Ty->getSizeInBits() / 8;'... These should also be fixed, I presume, but I don't see a getStoreSize() method or similar in Type...
>
> Also, with
>
>   diff --git a/include/llvm/IR/DebugInfoMetadata.h b/include/llvm/IR/DebugInfoMetadata.h
>   index c515f6d..a8aa195 100644
>   --- a/include/llvm/IR/DebugInfoMetadata.h
>   +++ b/include/llvm/IR/DebugInfoMetadata.h
>   @@ -594,6 +594,7 @@ public:
>      unsigned getLine() const { return Line; }
>      uint64_t getSizeInBits() const { return SizeInBits; }
>      uint32_t getAlignInBits() const { return AlignInBits; }
>   +  uint64_t getSizeInBytes() const { return (SizeInBits < 8 ? 1 : SizeInBits >> 3); }


Your way of implementing getSizeInBytes() is having some flaws:

1. for SizeInBits==0 it will return 1. <--- Seems wrong to me.
2. for SizeInBits > 8, but not a multiple of the byte size it rounds down (whereas it rounds up for < 8). <--- Inconsistent.

I think that a correct way of doing it would be

  uint64_t getSizeInBytes() const { return (SizeInBits + (ByteSizeInBits - 1) / ByteSizeInBits); }

where "ByteSizeInBits" is 8 for all in-tree-targets.


https://reviews.llvm.org/D40339