<div dir="ltr">Does anyone else have an opinion on verify.patch? Is it okay to require in the verifier that every entry in @llvm.used must have a name? Is it ever valid for an optimization to nuke the name of a global without first checking whether it's used by @llvm.used?<div>

<br></div><div style>I'm convinced that having an unnamed variable in @llvm.used can't possibly have any use (it must be dead, we can ignore/remove all such entries), but I'm not sure whether a well-behaved optimization ever produces them (just as well-behaved optimizations may cause blocks to become unreachable).</div>

<div><br></div><div><div class="gmail_extra" style>Review of verify.patch:<br><br></div><div class="gmail_extra" style>Please also update LangRef.html with this patch to mention that all entries in @llvm.used must have names.</div>

<div class="gmail_extra"><br></div><div class="gmail_extra">Review of global-opt.patch:<br></div><div class="gmail_extra" style><br></div><div class="gmail_extra" style>The + and - don't line up for some of this patch, making it a pain to review. Could you upload this to phab? <a href="http://llvm.org/docs/Phabricator.html">http://llvm.org/docs/Phabricator.html</a></div>

<div class="gmail_extra" style><br></div><div class="gmail_extra" style>+/// \brief Given "llvm.used" or "llvm.compiler_used" as a global nome, collect<br></div><div class="gmail_extra" style><br></div>

<div class="gmail_extra" style>typo, "nome".</div><div class="gmail_extra" style><br></div><div class="gmail_extra" style>Good news, I think I've reviewed this patch before? It looks like it's probably ready to be submitted.</div>

<div class="gmail_extra" style><br></div><div class="gmail_extra" style>Nick</div><div class="gmail_extra"><br><div class="gmail_quote">On 27 May 2013 16:25, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Now that the simple fix is safely in the 3.3 branch, I would like to<br>


revisit the more complete one.<br>
<br>
The first patch (verify.patch) just checks that every entry in<br>
llvm.used has a name. As far as I can tell this is a reasonable<br>
restriction since the language reference defines llvm.used in terms of<br>
an invisible reference.<br>
<br>
The second patch (global-opt.patch) is the actual change. Instead of a<br>
custom implementation of replaceAllUsesWith, we just call<br>
replaceAllUsesWith and recreate llvm.used and llvm.compiler-used.<br>
<br>
This change is particularity interesting because it makes llvm see<br>
through what clang is doing with static used functions in extern "C"<br>
contexts. With this change, running clang -O2 in<br>
<br>
extern "C" {<br>
  __attribute__((used)) static void foo() {}<br>
}<br>
<br>
produces<br>
<br>
@llvm.used = appending global [1 x i8*] [i8* bitcast (void ()* @foo to<br>
i8*)], section "llvm.metadata"<br>
define internal void @foo() #0 {<br>
entry:<br>
  ret void<br>
}<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div></div>