<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;"><div>Hi Anton, hi Duncan,</div><div><br></div><div>Sorry for the late reply, I was checking the information Anton’s pointed out and now, I need your opinions on what I saw.</div><div><br></div><div>So, according to the documentation, TypeInfos and more specifically the arguments of the clause part in landing pad instructions are supposed to be GlobalVariable.</div><div><br></div><div>Now, if you look at the code in the backend, the Verifier pass checks that the argument of the catch clause is Constant (which is a superclass of GlobalVariable and GEP) and that the type is a pointer type (Verifier::visitLandingPadInst).</div><div><br></div><div>Note: The type doesn’t change when the variable is merged (otherwise there is something wrong!).</div><div><br></div><div>Similarly, LLParser (LLParser::ParseLandingPad) and BitcodeReader (BitcodeReader::ParseFunctionBody) do not rely on GlobalVariable, they just check that the type is not an ArrayType.</div><div><br></div><div>Then, for the type info properly speaking, MachineModuleInfo uses GlobalVariable to map the type to an id (MachineModuleInfo::getTypeIDFor related) and AsmPrinter uses their name for printing.</div><div><br></div><div>As far as I can tell, it feels like it is no more a problem to merge the global constants (i.e., potentially changing a GlobalVariable into a GEP), even if they are related to EH. In particular, the type info is currently built using llvm::ExtractTypeInfo, which already takes a Value and do a stripPointerCast that gets a global variable even from the GEP and GEP is already a subclass of Constant like GlobalVariable.</div><div><br></div><div>Therefore, do you think it is still a problem to merge global constant?</div><div><br></div><div>One last thing, I don’t know if .ll file can hit that, since I don’t know if clang is doing much work on then, but clang also expects GlobalVariable for landing pad instruction in one very specific case for ObjC++ exception (PersonalityHasOnlyCXXUses). From my tests, ll files don’t get into that path but maybe I’m missing something.</div><div>What do you think?</div><div><br></div><div>Thanks for your help,</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 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; ">PS: dragon egg seems fine with Constant for clauses in landing pad instruction too!</div>
</div>
<br><div><div>On Mar 1, 2013, at 10:53 AM, Anton Korobeynikov <<a href="mailto:anton@korobeynikov.info">anton@korobeynikov.info</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>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>> 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 <qcolombet@apple.com> wrote:<br><br>Ping?<br><br>-Quentin<br><br>On Feb 27, 2013, at 9:55 AM, Quentin Colombet <qcolombet@apple.com> 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 <baldrick@free.fr> 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>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br><br>_______________________________________________<br>llvm-commits mailing list<br>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br><br>_______________________________________________<br>llvm-commits mailing list<br>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br><br>_______________________________________________<br>llvm-commits mailing list<br>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<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></blockquote><br><br><br>--<br>With best regards, Anton Korobeynikov<br>Faculty of Mathematics and Mechanics, Saint Petersburg State University</div></blockquote></div><br></body></html>