[llvm-commits] patch: mdnode .bc format change

[Nick Lewycky]

Devang Patel wrote:
> Nick,
> 
> On Fri, Jul 3, 2009 at 9:19 PM, Nick Lewycky<nicholas at mxc.ca> wrote:
>> This patch implements the new MDNode on-disk format for .bc files which we
>> would use if we supported instructions in MDNodes.
>>
>> This format is plainly inefficient when MDNodes only contain constants, but
>> I don't want to release 2.6 with a legacy file format that we need to
>> support.
>>
>> In the current format we store the MDNodes in the Constants block with
>> references to the other constants by number.
>>
>> The new format mentions the MDNodes in the Constant block but stores no data
>> about them other than the fact they exist.
> 
> Why not put MDNodes in METADATA_BLOCK ?

They are. Because they're constants they're numbered in the constant 
block but that has no information about their contents.

We need them to be numbered in the constants block so that the 
instructions which come thereafter have a value number to refer to.

You can think of the constants block as containing the 'forward 
declarations' of the mdnodes to come. Then the instructions are listed 
which may refer to any constants, then the metadata_block fills in the 
full definitions of the mdnodes which may refer to the constants and 
instructions.

> Other comments:
> 
> - I think WriteMetadata() is going to write MDNode multiple times.

Never. The METADATA_BLOCK contains:

   { node size, value number 1, value number 2 ... }

for each MDNode. Note that only the depth=1 level of an MDNode is 
included; any MDNodes it contains are referenced by value number. This 
even handles circular references correctly.

> - Reusing ConstantPlaceHolder for Metadata is not a good idea, I've
> doubts that this will work. ConstantPlaceHolder is meant for
> ConstExprs.

It works fine :) If you like I can refactor it into a 
MetadataPlaceHolder, but I don't see the benefit. What in particular are 
you worried about (ie. why does it matter that ConstantPlaceHolder was 
meant for ConstExprs)?

> I am moving in a direction where metadata is not derived from
> Constant. I will introduce new block in bitcode to hold metadata
> strings and nodes.

Okay. I assume this goes with removing the uniqueness property from MDNode?

Nick