[PATCH] Bitcode: Collect all MDString records into a single blob

[Duncan P. N. Exon Smith via llvm-commits]

> On 2016-Mar-24, at 16:46, Mehdi Amini <mehdi.amini at apple.com> wrote:
> 
> +        // FIXME: Avoid this upgrade logic; it's costly in the common case, and
> +        // incorrectly modifies all MDStrings.
> +        String = Current.str();
> +        llvm::UpgradeMDStringConstant(String);
> 
> Since we can't have old bitcode with METADATA_BULK_STRING_SIZES, and the only way to produce these deprecated "llvm.vectorizer.*" metadata is to write textual IR (for which we don't provide any guarantee). 
> So I'd advocate to drop it here (any lit test that is using the old syntax should be updated, bitcode files may be checked in with the old names for auto-upgrade test purpose, but that won't exercise this code path any way).

Fixed in r264373.

> +static unsigned createMDStringDataAbbrev(BitstreamWriter &Stream) {
> 
> There is one call site, for such a simple function are the other abbrev creation also using a separate function for creation?

Yes, the other metadata abbreviations are in functions.  I think it's
a cleaner model because both the `unsigned` and the `BitCodeAbbrev`
are rightly called some variation of "Abbrev".  With a separate
function you don't have to worry about which name refers to which
(since they are never in the same scope).

> +        return error("Invalid record");
> 
> The string could be more explicit (all of them).
> 
> Overall it is easy for me to understand because I have all the context. Someone that has no idea about what's going on would have hard time (OK admittedly the full state of the bitcode reader/writer is kind of like that, but that's not an excuse), so I think this could benefit some better comments here and there.

I was following the current error style.  You're right; no reason not
to set a new standard.

Please have another look.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Bitcode-Collect-all-MDString-records-into-a-singl-v3.patch
Type: application/octet-stream
Size: 17015 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160324/9355075d/attachment.obj>