<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 1, 2014 at 9:29 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On Thu, May 1, 2014 at 8:31 PM, Rafael Espíndola<br>
<<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
> OptimizeGlobalCtorsList<br>
><br>
> start the name with a lowercase.<br>
<br>
</div>Done. (I'm just moving them around, but since that modifies "svn<br>
blame" output already, renaming at the same time is a good idea.)<br>
<div class=""><br>
><br>
> +bool IsEmptyFunction(Function* F) {<br>
> +  BasicBlock &Entry = F->getEntryBlock();<br>
> +  return Entry.size() == 1 && isa<ReturnInst>(Entry.front());<br>
>  }<br>
><br>
> There is no way to cast to void in llvm, right? The thing I am afraid<br>
> of is if a trapping constant can be hidden in there.<br>
<br>
</div>global_ctors are always arrray of { i32, void ()* }…oooh I see what<br>
you're saying. I think it should be safe as-is, but I added a check<br>
for RI.getReturnValue() too.<br>
<div class=""><br>
> +/// Given a specified llvm.global_ctors list, install the<br>
> +/// specified array, returning the new global to use.<br>
> +void InstallGlobalCtors(GlobalVariable *GCL,<br>
><br>
> lowercase. Returning? install?<br>
<br>
</div>Done.<br>
<br>
> ParseGlobalCtors<br>
><br>
> lowercase.<br>
<br>
Done.<br>
<div class=""><br>
> +  if (!GV->hasUniqueInitializer()) return nullptr;<br>
><br>
> please clang-format.<br>
<br>
</div>Done.<br>
<div class=""><br>
> +    // Init priority must be standard.<br>
><br>
> Do you know why? At least for the empty case it should be valid to<br>
> drop constructors of any priority.<br>
<br>
</div>I don't know. Constructors with init priority are rare, and empty<br>
constructors with init priority are probably rarer still though.<br>
<div class=""><br>
> + ... Call "Remove" for<br>
><br>
><br>
> It is actually a predicate, so maybe ShouldRemove?<br>
<br>
</div>Done. I also changed this to a function pointer with a context<br>
argument, since<br>
<a href="http://llvm.org/docs/CodingStandards.html#supported-c-11-language-and-library-features" target="_blank">http://llvm.org/docs/CodingStandards.html#supported-c-11-language-and-library-features</a><br>
says that std::function cannot be used yet.</blockquote><div><br></div><div>It looks like you don't actually want std::function (you don't want an owned and managed copy of a callable), you just want a non-owning, type-erasing callable reference. I wanted one of those about a month ago, and wrote a special-case handler for it at the time, but you've motivated me to implement the general case.</div>
<div><br></div><div>Attached is a patch for an llvm::function_ref (a non-owning, type-erased callable) and a sample use. This would be usable as a drop-in replacement for std::function in your previous patch.</div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> This is a case where an integration test might be a good idea.  Do you<br>
> have a reasonably size test where this change causes a ctor to  be<br>
> deleted at -O2? We might want to test that, so that even if we go with<br>
> a different optimization strategy in the future we still have a test<br>
> of the end result (the ctor gets deleted somewhere in -O2).<br>
<br>
</div>Yes, there's one in the bug. I can check that in on the clang side<br>
once this patch is in.<br>
<br>
Thanks!<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>
<br></blockquote></div><br></div></div>