IR: Add distinct MDNodes as a first-class concept

Pete Cooper peter_cooper at apple.com
Wed Jan 7 16:02:54 PST 2015


Hi Duncan

I assume that prior to these commits clang is already using MDNode::getDistinct?  Otherwise I can’t see how your clang patch would work here.

Otherwise LGTM.

Thanks,
Pete
> On Jan 7, 2015, at 3:27 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> In order to "defeat" `MDNode` uniquing, metadata schemas resort to
> using self-references to prevent merging.
> 
> Introduce a `distinct` designator which explicitly requests this
> behaviour, without the need for a self-reference.  These nodes are
> still stored in the `LLVMContext`, but are not uniqued based on
> their operands (well, not uniqued at all).
> 
>  - This concept *already exists*.  Before the metadata/value split,
>    it was used whenever operands went to null (to prevent teardown
>    madness, but it happened more frequently).  It's still in use
>    for self-referencing `MDNode`s, as well as when
>    `MDNode::replaceOperandWith()` causes a uniquing collision (see
>    module flags behaviour before r225397 for an example).
> 
>    Some recent commits have already exposed `getDistinct()` and
>    `isDistinct()` as API (see r225401 and r225406).
> 
>  - The first patch adds assembly/bitcode support.  I chose the
>    'distinct' keyword.  It has an accompanying clang patch to
>    update testcases.
> 
>  - The second patch adds support to `MapMetadata()` to maintain the
>    'distinct'-ness.
> 
>  - The third patch just clears out the TODO.
> 
> I haven't updated any metadata schemas here; I figure the owners of
> the schemas can update those in their own time.
> 
> My main concern here was about how to deal with `MapMetadata()` --
> it feels wrong to just duplicate everything.  However, I looked
> through all the callers.  In *most* cases RF_NoModuleLevelChanges
> is passed, and in the others this naive solution seems correct (or
> at least harmless).
> 
> If this becomes an issue (i.e., there's a place with module-level
> changes where distinct metadata should only be duplicated when
> operands change), we can add another flag to decide whether to
> duplicate it.  For now, this just matches the behaviour we already
> get out of self-references.
> 
> <0001-IR-Add-distinct-MDNodes-to-bitcode-and-assembly.patch><clang.patch><0002-Utils-Keep-distinct-MDNodes-distinct-in-MapMetadata.patch><0003-IR-Drop-TODO-now-that-PR22111-is-finished.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list