<div dir="ltr">Thanks for the feedback. I think the duplication is fairly benign given the proximity of the tests to one another, but I don't have strong feeling one way or the other - please feel free to refactor if you think it would be an improvement.<div>
<br></div><div style>- Lang.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Mar 4, 2013 at 2:53 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Mar 4, 2013 at 2:40 PM, Lang Hames <<a href="mailto:lhames@gmail.com">lhames@gmail.com</a>> wrote:<br>

> Author: lhames<br>
> Date: Mon Mar  4 16:40:44 2013<br>
> New Revision: 176459<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=176459&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=176459&view=rev</a><br>
> Log:<br>
> Check isDiscardableIfUnused, rather than hasLocalLinkage, when bumping<br>
> GlobalValue linkage up to ExternalLinkage in the ExtractGV pass. This<br>
> prevents linkonce and linkonce_odr symbols from being DCE'd.<br>
><br>
> Added:<br>
>     llvm/trunk/test/Other/extract-linkonce.ll<br>
> Modified:<br>
>     llvm/trunk/lib/Transforms/IPO/ExtractGV.cpp<br>
><br>
> Modified: llvm/trunk/lib/Transforms/IPO/ExtractGV.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/ExtractGV.cpp?rev=176459&r1=176458&r2=176459&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/ExtractGV.cpp?rev=176459&r1=176458&r2=176459&view=diff</a><br>

> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/IPO/ExtractGV.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/IPO/ExtractGV.cpp Mon Mar  4 16:40:44 2013<br>
> @@ -60,7 +60,7 @@ namespace {<br>
>              continue;<br>
>          }<br>
><br>
> -        bool Local = I->hasLocalLinkage();<br>
> +        bool Local = I->isDiscardableIfUnused();<br>
>          if (Local)<br>
>            I->setVisibility(GlobalValue::HiddenVisibility);<br>
><br>
> @@ -80,7 +80,7 @@ namespace {<br>
>              continue;<br>
>          }<br>
><br>
> -        bool Local = I->hasLocalLinkage();<br>
> +        bool Local = I->isDiscardableIfUnused();<br>
>          if (Local)<br>
>            I->setVisibility(GlobalValue::HiddenVisibility);<br>
><br>
> @@ -97,7 +97,7 @@ namespace {<br>
>          Module::alias_iterator CurI = I;<br>
>          ++I;<br>
><br>
> -        if (CurI->hasLocalLinkage()) {<br>
> +        if (CurI->isDiscardableIfUnused()) {<br>
>            CurI->setVisibility(GlobalValue::HiddenVisibility);<br>
<br>
</div></div>Little bit of deja vu? Is there some refactoring that could be done to<br>
reduce the duplication in this code? (I haven't really looked in<br>
detail, just saw a patch with three rather similar changes to rather<br>
similar code)<br>
<div class="HOEnZb"><div class="h5"><br>
>            CurI->setLinkage(GlobalValue::ExternalLinkage);<br>
>          }<br>
><br>
> Added: llvm/trunk/test/Other/extract-linkonce.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/extract-linkonce.ll?rev=176459&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/extract-linkonce.ll?rev=176459&view=auto</a><br>

> ==============================================================================<br>
> --- llvm/trunk/test/Other/extract-linkonce.ll (added)<br>
> +++ llvm/trunk/test/Other/extract-linkonce.ll Mon Mar  4 16:40:44 2013<br>
> @@ -0,0 +1,23 @@<br>
> +; RUN: llvm-extract -func foo -S < %s | FileCheck %s<br>
> +; RUN: llvm-extract -delete -func foo -S < %s | FileCheck --check-prefix=DELETE %s<br>
> +<br>
> +; Test that we don't convert weak_odr to external definitions.<br>
> +<br>
> +; CHECK:      @bar = external hidden global i32<br>
> +; CHECK:      define hidden i32* @foo() {<br>
> +; CHECK-NEXT:  ret i32* @bar<br>
> +; CHECK-NEXT: }<br>
> +<br>
> +; DELETE: @bar = hidden global i32 42<br>
> +; DELETE: declare hidden i32* @foo()<br>
> +<br>
> +@bar = linkonce global i32 42<br>
> +<br>
> +define linkonce i32* @foo() {<br>
> +  ret i32* @bar<br>
> +}<br>
> +<br>
> +define void @g() {<br>
> +  call i32* @foo()<br>
> +  ret void<br>
> +}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>