<div dir="ltr"><div>On Mon, Jan 11, 2016 at 8:53 PM, David Blaikie via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</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"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Fri, Dec 18, 2015 at 6:39 AM, Joerg Sonnenberger via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</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">On Fri, Dec 18, 2015 at 09:17:17AM +0000, Dmitry Polukhin via llvm-commits wrote:<br>> Just for the sake of completeness, there is a way to avoid IR extension<br>> - it is possible to emit global asm statement like<br>> `__asm__ (".type resolver_alias_name, @gnu_indirect_function")`<br>> and avoid IR extension but I didn't seriously consider this path.<br><br>Why not? Let me put it another way: what is the advantage of making the<br>intermediate layers aware of ifuncs? Can they provide better<br>optimisations? Can they diagnose any potential bugs?</blockquote><div><br></div></span><div>One possible optimization:<br><br>say you have a target specific function that calls a generic function that in turn calls an ifunc function<br>with ifunc as a first-class construct you can potentially inline both calls into the target specific outer function - inline the generic one, and in doing so, observe that the resolver function would produce a constant result (would have to teach the inliner about a cpuid intrinsic of some sort) & thus lower the call-to-ifunc with a direct call, then inline that call, etc.<br></div></div></div></div></blockquote></div><div><br></div><div>Also I'm not very confident that global asm state will be enough to keep alias in-place. It looks like it might be optimized away in some cases because LLVM parses global asm statements very late and doesn't know their content during optimizations. Final goal is to support attribute target with possible inlining etc. proposed approach with explicit ifunc support in IR is much better in long term.</div><div><br></div><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 12, 2016 at 12:00 AM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.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"><div class=""><div class="h5"><span style="color:rgb(34,34,34)">> Index: lib/Transforms/IPO/GlobalDCE.</span><span style="color:rgb(34,34,34)">cpp</span><br></div></div>
> ===================================================================<br>
> --- lib/Transforms/IPO/GlobalDCE.cpp<br>
> +++ lib/Transforms/IPO/GlobalDCE.cpp<br>
> @@ -205,9 +205,9 @@<br>
> // referenced by the initializer to the alive set.<br>
> if (GV->hasInitializer())<br>
> MarkUsedGlobalsAsNeeded(GV->getInitializer());<br>
> - } else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(G)) {<br>
> - // The target of a global alias is needed.<br>
> - MarkUsedGlobalsAsNeeded(GA->getAliasee());<br>
> + } else if (GlobalIndirectSymbol *GIS = dyn_cast<GlobalIndirectSymbol>(G)) {<br>
> + // The target of a global alias or ifunc is needed.<br>
> + MarkUsedGlobalsAsNeeded(GIS->getIndirectSymbol());<br>
<br>
It seems like this change could be done more incrementally. For<br>
example, if you add `GlobalIndirectSymbol` in an initial commit, you can<br>
then convert all the appropriate things from `GlobalAlias` to<br>
`GlobalIndirectSymbol` one at a time, before finally landing the new<br>
`GlobalIFunc` commit which adds the new IR type.<br>
<br>
(I'm just picking on the first such change in the patch.)<br></blockquote><div><br></div><div>I think it is good to have the whole path in single place to have complete picture of proposed change. But I'm ready to split it in as many parts as needed to speed up code review. My only worry that I will have to touch some files more than once with this approach and it won't actually speed up or simplify things. Please let me know if you really want me to split this patch in multiple patches.</div><div><br></div><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">
> } else {<br>
> // Otherwise this must be a function object. We have to scan the body of<br>
> // the function looking for constants and global values which are used as<br>
> Index: lib/IR/AsmWriter.cpp<br>
> ===================================================================<br>
> --- lib/IR/AsmWriter.cpp<br>
> +++ lib/IR/AsmWriter.cpp<br>
> @@ -249,11 +254,15 @@<br>
> predictValueUseListOrder(&F, nullptr, OM, Stack);<br>
> for (const GlobalAlias &A : M->aliases())<br>
> predictValueUseListOrder(&A, nullptr, OM, Stack);<br>
<span class="">> + for (const GlobalIFunc &I : M->ifuncs())<br>
</span>> + predictValueUseListOrder(&I, nullptr, OM, Stack);<br>
> for (const GlobalVariable &G : M->globals())<br>
> if (G.hasInitializer())<br>
> predictValueUseListOrder(G.getInitializer(), nullptr, OM, Stack);<br>
> for (const GlobalAlias &A : M->aliases())<br>
> predictValueUseListOrder(A.getAliasee(), nullptr, OM, Stack);<br>
<span class="">> + for (const GlobalIFunc &I : M->ifuncs())<br>
</span>> + predictValueUseListOrder(I.getResolver(), nullptr, OM, Stack);<br>
<br>
Can you add a lit test (if there isn't one already) that calls<br>
verify-uselistorder in a way that would confirm this "predict" call was<br>
added in the right place?<br>
<br>
I think it should be sufficient to a GIF that's referenced by each of<br>
GlobalVariable, GlobalAlias, GlobalIFunc, and Function; and a Function<br>
that's referenced in the same four ways.<br></blockquote><div><br></div><div>I added such test, please check that it is what you were asking for.</div><div><br></div></div></div></div>