IR: Add distinct MDNodes as a first-class concept

[Duncan P. N. Exon Smith]

> On 2015-Jan-07, at 16:06, Pete Cooper <peter_cooper at apple.com> wrote:
> 
>> 
>> On Jan 7, 2015, at 4:04 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> Actually, no -- see below:
>> 
>>> On 2015-Jan-07, at 16:02, Pete Cooper <peter_cooper at apple.com> wrote:
>>> 
>>> 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).
>> 
>> ^ Distinct `MDNode`s are implicitly created for self-references and on
>> uniquing collisions.
> Cool.  Still LGTM then :)

Thanks for the review!

r225475
r225476
r225477


>> 
>>>> 
>>>>  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