<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi David,<div><br></div><div>I was looking at Duncan’s proposal regarding the creation of GlobalAlias to GEP with non zero offset for merged global variables.</div><div>Based on Duncan’s comment, I’ve been able to find a PR that you initiated on that:</div><div><a href="http://www.llvm.org/bugs/show_bug.cgi?id=4739">http://www.llvm.org/bugs/show_bug.cgi?id=4739</a></div><div><br></div><div>The resolution of this PR was INVALID.</div><div><br></div><div>If I understand correctly this PR, it is not possible to create aliases to the middle of a structure for Mach-O target.</div><div>What is the current status for other targets?</div><div><br></div><div>Also, what do you think would be a viable approach for the global merge problem?</div><div><br></div><div>Thanks,</div><div><br><div apple-content-edited="true">
<div style="color: rgb(0, 0, 0); font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">-Quentin</div>
</div>
<br><div><div>On Mar 12, 2013, at 10:16 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Duncan,<div><br></div><div>Thanks for the insightful information!</div><div><br></div><div>If I've understood correctly what you’re saying, the current process of the global merge pass is wrong since it may merge globals referenced in inline assembly statement.</div><div><br></div><div>If yes, we should file a PR!</div><div><br></div><div>I like you’re idea with aliases.</div><div><br></div><div>Is there a way to know if aliases and this kind of aliases are supported on the current platforms?</div><div><br></div><div>Assuming I can use aliases to achieve a valid global merging, it looks like it is incompatible with the spirit of <a href="http://llvm.org/bugs/show_bug.cgi?id=10367">http://llvm.org/bugs/show_bug.cgi?id=10367</a>.</div><div><br></div><div>What do you think?</div><div><br></div><div>Thanks,</div><div><br><div apple-content-edited="true"><div style="font-family: Helvetica; font-size: medium; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">-Quentin</div></div><br><div><div>On Mar 12, 2013, at 1:07 AM, Duncan Sands <<a href="mailto:baldrick@free.fr">baldrick@free.fr</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">Hi Quentin,<br><br><blockquote type="cite">Sorry for the late reply, I was checking the information Anton’s pointed out and<br>now, I need your opinions on what I saw.<br><br>So, according to the documentation, TypeInfos and more specifically the<br>arguments of the clause part in landing pad instructions are supposed to be<br>GlobalVariable.<br></blockquote><br>I'm pretty sure it needs to be a GlobalVariable or a bitcast of one. In short<br>something which turns into a simple label in the assembler.<br><br><blockquote type="cite"><br>Now, if you look at the code in the backend, the Verifier pass checks that the<br>argument of the catch clause is Constant (which is a superclass of<br>GlobalVariable and GEP) and that the type is a pointer type<br>(Verifier::visitLandingPadInst).<br></blockquote><br>At the IR level it doesn't matter much what a typeinfo is. The restrictions are<br>coming from codegen.<br><br><blockquote type="cite"><br>Note: The type doesn’t change when the variable is merged (otherwise there is<br>something wrong!).<br><br>Similarly, LLParser (LLParser::ParseLandingPad) and BitcodeReader<br>(BitcodeReader::ParseFunctionBody) do not rely on GlobalVariable, they just<br>check that the type is not an ArrayType.<br><br>Then, for the type info properly speaking, MachineModuleInfo uses GlobalVariable<br>to map the type to an id (MachineModuleInfo::getTypeIDFor related) and<br>AsmPrinter uses their name for printing.<br><br>As far as I can tell, it feels like it is no more a problem to merge the global<br>constants (i.e., potentially changing a GlobalVariable into a GEP), even if they<br>are related to EH. In particular, the type info is currently built using<br>llvm::ExtractTypeInfo, which already takes a Value and do a stripPointerCast<br>that gets a global variable even from the GEP and GEP is already a subclass of<br>Constant like GlobalVariable.<br><br>Therefore, do you think it is still a problem to merge global constant?<br></blockquote><br>Yes, as I mentioned above I think the typeinfo needs to be a GlobalVariable<br>(or a bitcast of one) not an offset into one (GEP). See below for some thoughts<br>on how to relax this.<br><br><blockquote type="cite"><br>One last thing, I don’t know if .ll file can hit that, since I don’t know if<br>clang is doing much work on then, but clang also expects GlobalVariable for<br>landing pad instruction in one very specific case for ObjC++ exception<br>(PersonalityHasOnlyCXXUses). From my tests, ll files don’t get into that path<br>but maybe I’m missing something.<br>What do you think?<br><br>Thanks for your help,<br><br>-Quentin<br>PS: dragon egg seems fine with Constant for clauses in landing pad instruction too!<br></blockquote><br>Front-ends know what they produce, they aren't really relevant here. Dragonegg<br>produces (bitcasts of) GlobalVariables. Using Constant is just a convenient way<br>of handling both GlobalVariables and bitcasts of global variables in a uniform<br>way.<br><br>Note that it isn't only exception handling typeinfos that is an issue here:<br>people using a GlobalVariable G in inline asm may well refer to it by its<br>name "G" in the inline asm. Merging globals could break this.<br><br>Here is a possible solution to all these issues:<br><br>(1) Enhance aliases so that they can be offsets into a global, not just an alias<br>to a global (there is a PR requesting this somewhere);<br>(2) When merging a global G into a mega-global, turn "G" into an alias pointing<br>to the position in the mega global.<br><br>Eg:<br><br>Before:<br><br> @F = global...<br> @G = global...<br> @H = global...<br><br>After:<br><br> @Merged = global ... contains F, G then H<br> @F = alias bitcast @Merged to ...<br> @G = alias getelementptr @Merged, ...<br> @H = alias getelementptr @Merged, ...<br><br>This should then result in F, G and H turning up as labels in the assembler,<br>usable as typeinfos. It would likewise solve the inline asm issue.<br><br>The problems are: I'm not sure aliases are supported on all platforms, and<br>I'm even less sure which platforms support this kind of alias.<br><br>Ciao, Duncan.<br><br><br><blockquote type="cite"><br>On Mar 1, 2013, at 10:53 AM, Anton Korobeynikov <<a href="mailto:anton@korobeynikov.info">anton@korobeynikov.info</a><br><<a href="mailto:anton@korobeynikov.info">mailto:anton@korobeynikov.info</a>>> wrote:<br><br><blockquote type="cite">Hi Quentin,<br><br>Indeed, GlobalMerge did not merge global constants because EH<br>processing code expected typeinfo and other stuff to be globals, not<br>gep's.<br><br>The situation changed with addition of landingpad instruction. You<br>might want to check how it handle GEPs for typeinfos (though, I'd say<br>that LangRef states that typeinfo is a global).<br><br><br>On Fri, Mar 1, 2013 at 10:05 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a><br><<a href="mailto:qcolombet@apple.com">mailto:qcolombet@apple.com</a>>> wrote:<br><blockquote type="cite">Hi Duncan,<br><br>I've checked in LLVM documentation and the unnamed_addr marker is not<br>specific to global variables that are constant. Hence, based on your<br>comment, I was thinking that the global merge pass may perform illegal<br>transformations.<br>Actually, I thought about it and from my understanding, merging two globals<br>like global merge pass does is not incompatible with having two distinct<br>addresses.<br>Indeed, global merge keeps all globals but wraps them into a structure.<br>E.g.,<br>int a;<br>int b;<br>=><br>struct {<br>int a;<br>int b;<br>} MergedGV;<br><br>Thus, &MergedGV.a != &MergedGV.b<br><br>This brings me back to my initial question, does gathering constants into<br>structures still cause a problem with EH since it does not seem incompatible<br>with unnamed_addr?<br><br>Note that I've run the test case that was triggering the failure, and it<br>works with my patch enabled. But since I don't know how EH are handled I<br>prefer to have an external opinion.<br><br>Reference of the test case triggering the failure:<br><a href="http://llvm.org/bugs/show_bug.cgi?id=7716">http://llvm.org/bugs/show_bug.cgi?id=7716</a><br><br>Thanks,<br><br>-Quentin<br><br>On Feb 28, 2013, at 10:08 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br>Ping?<br><br>-Quentin<br><br>On Feb 27, 2013, at 9:55 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br>Hi Duncan,<br><br>Does it mean that the current behavior of global merge is wrong or is this<br>unnamed_addr marker apply only to global constants?<br><br>Anyway, thanks for the information.<br><br>-Quentin<br><br>On Feb 27, 2013, at 12:50 AM, Duncan Sands <<a href="mailto:baldrick@free.fr">baldrick@free.fr</a>> wrote:<br><br>Hi Quentin, you should only be merging them if they are marked with<br>unnamed_addr (this indicates that no-one is expecting the globals<br>to have distinct addressses). As EH type infos shouldn't get<br>unnamed_addr, that should mean that the EH issue isn't a problem any<br>more.<br><br>Ciao, Duncan.<br><br>You will find enclosed a patch that adds the support of constant variables<br>for<br>the generic global merge pass.<br>I've also attached a motivating example, test.c.<br><br>In that file there are several global variables accessed within the same<br>function.<br>If all the variables are non-const, global merge generates one unique<br>variable,<br>which translates into one unique load in the assembly file.<br><br>clang -arch arm -O3 test.c -o - -S<br><br>If some are const (the example is very stupid), global merge does not merge<br>anything and one load for each variable is issued, i.e., 3<br>clang -arch arm -O3 test.c -o - -S -DWITHCONST<br><br>If global merge had merged the constant globals, only 2 loads would have<br>been<br>issued. The patch allows to do that.<br><br>Although the patch is fairly simple, it was not done before because it seems<br>it<br>was breaking the EH processing (see comment in the original code).<br>Is it still true?<br><br>If yes, how could I reproduce that?<br><br>Note: that the patch disables by default the handling of global constants<br>because the current heuristic may generate worse code. A tuning may be<br>necessary, but it would be done as a second step<br><br>-Quentin<br><br><br><br><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">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><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">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><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">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><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">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span><<a href="mailto:llvm-commits@cs.uiuc.edu">mailto:llvm-commits@cs.uiuc.edu</a>><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br></blockquote><br><br><br>--<br>With best regards, Anton Korobeynikov<br>Faculty of Mathematics and Mechanics, Saint Petersburg State University</blockquote></blockquote></div></blockquote></div><br></div>_______________________________________________<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">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div><br></div></body></html>