[llvm] r243961 - Linker: Fix references to uniqued nodes after r243883

Duncan P. N. Exon Smith dexonsmith at apple.com
Tue Aug 4 06:26:26 PDT 2015


> On 2015-Aug-04, at 00:50, Justin Bogner <mail at justinbogner.com> wrote:
> 
> "Duncan P. N. Exon Smith" <dexonsmith at apple.com> writes:
>> Author: dexonsmith
>> Date: Tue Aug  4 01:42:31 2015
>> New Revision: 243961
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=243961&view=rev
>> Log:
>> Linker: Fix references to uniqued nodes after r243883
>> 
>> r243883 started moving 'distinct' nodes instead of duplicated them in
>> lib/Linker.  This had the side-effect of sometimes not cloning uniqued
>> nodes that reference them.  I missed a corner case:
>> 
>>    !named = !{!0}
>>    !0 = !{!1}
>>    !1 = distinct !{!0}
>> 
>> !0 is the entry point for "remapping", and a temporary clone (say,
>> !0-temp) is created and mapped in case we need to model a uniquing
>> cycle.
>> 
>>    Recursive descent into !1.  !1 is distinct, so we leave it alone,
>>    but update its operand to !0-temp.
>> 
>> Pop back out to !0.  Its only operand, !1, hasn't changed, so we don't
>> need to use !0-temp.  !0-temp goes out of scope, and we're finished
>> remapping, but we're left with:
>> 
>>    !named = !{!0}
>>    !0 = !{!1}
>>    !1 = distinct !{null} ; uh oh...
>> 
>> Previously, if !0 and !0-temp ended up with identical operands, then
>> !0-temp couldn't have been referenced at all.  Now that distinct nodes
>> don't get duplicated, that assumption is invalid.  We need to
>> !0-temp->replaceAllUsesWith(!0) before freeing !0-temp.
>> 
>> I found this while running an internal `-flto -g` bootstrap.  Strangely,
>> there was no case of this in the open source bootstrap I'd done before
>> commit...
> 
> You probably got a mail from the bot, but if not it looks like msan is
> unhappy about this change:
> 
>  http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/6041/steps/check-llvm%20msan/logs/stdio
> 

Thanks, r243976.

>> 
>> Added:
>>    llvm/trunk/test/Linker/uniqued-distinct-cycles.ll
>> Modified:
>>    llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
>> 
>> Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=243961&r1=243960&r2=243961&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Tue Aug  4 01:42:31 2015
>> @@ -258,9 +258,11 @@ static Metadata *mapUniquedNode(const MD
>> 
>>   // Create a temporary node upfront in case we have a metadata cycle.
>>   auto ClonedMD = Node->clone();
>> -  if (!remap(Node, ClonedMD.get(), Cycles, VM, Flags, TypeMapper, Materializer))
>> +  if (!remap(Node, ClonedMD.get(), Cycles, VM, Flags, TypeMapper, Materializer)) {
> 
> 80-col?

Somehow missed this until I was about to reply in my excitement about
the ASan failure.  r243977.

>>     // No operands changed, so use the identity mapping.
>> +    ClonedMD->replaceAllUsesWith(const_cast<MDNode *>(Node));
>>     return mapToSelf(VM, Node);
>> +  }
>> 
>>   // At least one operand has changed, so uniquify the cloned node.
>>   return mapToMetadata(VM, Node,
>> 
>> Added: llvm/trunk/test/Linker/uniqued-distinct-cycles.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/uniqued-distinct-cycles.ll?rev=243961&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Linker/uniqued-distinct-cycles.ll (added)
>> +++ llvm/trunk/test/Linker/uniqued-distinct-cycles.ll Tue Aug  4 01:42:31 2015
>> @@ -0,0 +1,14 @@
>> +; RUN: llvm-link -o - %s | llvm-dis | FileCheck %s
>> +
>> +; CHECK: !named = !{!0, !2}
>> +!named = !{!0, !2}
>> +
>> +; CHECK:      !0 = !{!1}
>> +; CHECK-NEXT: !1 = distinct !{!0}
>> +!0 = !{!1}
>> +!1 = distinct !{!0}
>> +
>> +; CHECK-NEXT: !2 = distinct !{!3}
>> +; CHECK-NEXT: !3 = !{!2}
>> +!2 = distinct !{!3}
>> +!3 = !{!2}
>> 
>> 
>> _______________________________________________
>> 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