[LLVMdev] [proposal] Extensible IR metadata
devang.patel at gmail.com
Wed Sep 16 10:11:33 PDT 2009
On Wed, Sep 16, 2009 at 9:53 AM, Jeffrey Yasskin <jyasskin at google.com> wrote:
> On Tue, Sep 15, 2009 at 11:37 PM, Devang Patel <devang.patel at gmail.com> wrote:
>> On Fri, Sep 11, 2009 at 9:57 AM, Chris Lattner <clattner at apple.com> wrote:
>>> Devang's work on debug info prompted this, thoughts welcome:
>> Here is partial initial implementation.
>> Thoughts ?
> setHasMetadata probably shouldn't be public, like there's no way to
> directly set HasValueHandle.
that means making llvm::Metadata a friend of llvm::Value. fine.
> I'd make the MDKindId type (or typedef) now so it's obvious what those
> "unsigned"s mean.
> Should the metadata name->id map be per-context or global? I think
> it's unlikely people will be registering these things during
> compilation, so we can afford the overhead of a lock, and making it
> global helps with the wrapper types (avoids one vector lookup). But I
> can also change it when I add the wrapper types.
> It's too bad we don't have a SmallMap class to give MDInfoTy a good
> API and make it efficient at both small and large map sizes. Maybe
> s/MDInfoTy/MDMapTy/ to point out that it's a map from MDKindIds to
> Comment whether RegisterMDKind requires that its argument is
> as-yet-unknown in Metadata.h.
> Context.pImpl->TheMetadata.ValueIsDeleted(this); in ~Value will always
> call the empty ValueIsDeleted(Value*) overload.
> Please write a unittest for this.
> Metadata::getMDKind(Name) can just be "return MDHandlerNames.lookup(Name)"
> Could you comment what the WeakVH in MDPairTy is following? I got
> confused and thought it was tracking the Instruction holding the
> metadata rather than the MDNode.
Instruction can hold either MDNode or MDString. MDNode can be replaced
transparently by the MDNode uniquing scheme, so its holder should
always use WeakVH.
> You did say this was partial; sorry if I pointed out anything you were
> already planning to do.
I meant, this only covers basic metadata storage. Change required in
llvm parser/printer, bitcode reader/writer, IR builder and test cases
are missing :)
More information about the llvm-dev