[PATCH] D41695: [Metadata] Extend 'count' field of DISubrange to take a metadata node

[David Blaikie via llvm-commits]

On Wed, Jan 17, 2018 at 10:12 AM Adrian Prantl via Phabricator <
reviews at reviews.llvm.org> wrote:

> aprantl added a comment.
>
> It looks like this changes the layout of DISubrange from
>
> Flags | Count |
>
>
>
> ================
> Comment at: include/llvm/IR/DebugInfoMetadata.h:372
> +
> +  ConstantInt *getCount() const {
> +    if (auto *MD = dyn_cast<ConstantAsMetadata>(getRawCountNode()))
> ----------------
> Perhaps:
> `Optional<ConstantInt *> getConstantCount()` ?
>

FWIW (& opinions vary, this isn't something that's clearly one way or the
other I think) - I generally use an existing empty state if there is one,
like nullptr. Adding an extra empty state seems likely to be confusing - if
I see Optional<T*> then I assume there's None, null, and the non-null
values - and have to think about what the difference might be between None
and null.


>
>
> ================
> Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:1445
>    Record.push_back(N->isDistinct());
> -  Record.push_back(N->getCount());
> +  Record.push_back(-1); // this field has become obsolete
> +  Record.push_back(VE.getMetadataOrNullID(N->getRawCountNode()));
> ----------------
> We typically version bitcode by adding a bit to the flags, like this:
> ` Record.push_back(N->isDistinct() | 1<<1);`
> This way we don't have to waste the space for the obsoleted field.
>
>
> https://reviews.llvm.org/D41695
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180117/bde8867f/attachment.html>