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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In https://reviews.llvm.org/D41695#976967, @sdesmalen wrote:

> I did some further investigation on the memory size impact of having an integer 'Count' field being replaced by a ConstantAsMetadata node in DISubrange. As suggested by @rnk, I counted the number of DISubrange objects in a compilation unit for builds of Clang, BLAS, and LAPACK.
>
> For a shared lib LTO build of Clang (for some reason I couldn't get it to work with static libs), the high-water mark of DISubrange occurrences found in one of the bigger libraries was <90. sizeof(ConstantAsMetadata) gives me 144 bytes, so this would be around 14kb of memory, which in comparison with the whole LTO Debug build feels negligable.


I'm totally satisfied with this reasoning. Thanks for checking, sorry to hold it up!

> BLAS didn't result in many DISubranges because the Fortran code uses more advanced arrays than can be described with a DISubrange using a constant count. I guess similar arguments will hold for most codes, with C/C++ using pointers in most places to 'describe' arrays. An LTO build of LAPACK resulted in a high-water mark of 45 DISubrange objects.
> 
> Perhaps I can collect more data, but I'm not so worried about replacing an integer Count with a ConstantAsMetadata node for the following reasons:
> 
> - Similar DISubrange objects are uniqued (see test/Bitcode/disubrange.ll) and I expect many similar ranges to be merged.
> - Most array-heavy C/C++ libraries will work with pointers and malloc, and not define much 'static' arrays with constant bounds, so most types won't have their bounds described in DISubrange anyway.
> - For code or frontends that extensively use the extended 'count' field of DISubrange (as implemented in this patch), it will likely reference different Metadata nodes rather than using a constant to express some of the more advanced array types. In that case, the memory cost will be justified by the extended functionality.
> 
>   The code can be changed to optimize the constant-value case, but since the impact seems small I propose to just keep it as a ConstantAsMetadata node as this keeps the code a bit simpler. Any objections to that?

No, I don't think the complexity of the optimization is worth it.


https://reviews.llvm.org/D41695