<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 Eric,</div><div><br></div><div>I know you are busy and I appreciate you are planning to have a look on that.</div><div>A question though, when do you think you will get a chance to look at that?</div><div><br></div><div>Thanks,</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 Apr 23, 2013, at 9:19 AM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.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-stroke-width: 0px;">I haven't forgotten :)<br><br>I'll try to get you a reply tomorrow.<br><br>-eric<br><br>On Tue, Apr 23, 2013 at 5:16 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><blockquote type="cite">Ping?<br><br>-Quentin<br><br>On Apr 19, 2013, at 11:08 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br>Hi Eric,<br><br>I’ve looked closer on the debug information were generated for the merged<br>variables.<br>It appeared that at the moment GlobalMerge has to perform its modifications<br>in doInitialization, because it relies on how the global variables will be<br>interpreted in DwarfDebug::beginModule which is called from the<br>doInitialization of the AsmPrinter pass.<br>More specifically, CompileUnit::createGlobalVariableDIE has a special case<br>for MergedGlobals:<br>[…]<br>if (const ConstantExpr *CE = getMergedGlobalExpr(N->getOperand(11)))<br><br>After a (very) quick look to the debug interfaces, I didn’t see a way to<br>reproduce this behavior a posteriori.<br><br>Any ideas?<br><br>Thanks,<br><br>-Quentin<br><br>On Apr 17, 2013, at 9:31 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br>Thanks Eric.<br><br>Do you think it would be possible/desirable to fix the debug information<br>within GlobalMerge?<br><br>Currently GlobalMerge pass is added to the pass manager in code that is ARM<br>specific, thus, I don’t have much latitude to move it. In particular, if I<br>move it at the earliest I can in target specific code (i.e., using<br>addIRPasses) I didn’t reproduce the old behavior.<br><br>-Quentin<br><br>On Apr 16, 2013, at 7:18 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br><br>On Tue, Apr 16, 2013 at 5:51 PM, Jim Grosbach <<a href="mailto:grosbach@apple.com">grosbach@apple.com</a>> wrote:<br><br><br>On Apr 16, 2013, at 5:48 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br>Oh, that’s true the pass is not run where it was running.<br>Thanks Jim.<br><br>I’ll have to check if we can reproduce the old behavior since the pass may<br>rely on running before the doInitialization of another pass.<br><br>Regarding the test cases, the debug information have changed.<br>MergedGlobals is no more referenced in the debug information. I’m fine with<br>that since it was not in the program initially (we give insight on something<br>the developer never wrote!), but it seems to be the expected behavior.<br><br><br>Interesting. Well, it’s entirely possible that expected != correct here. If<br>we’re referring back (correctly, of course) to the user’s variables and not<br>referencing the merged globals variable at all, that seems an improvement to<br>me. Eric, any thoughts on that?<br><br><br>The new code is, unfortunately, incorrect. The addresses of the<br>various objects are relocations for undefined symbols in the output<br>_x1, _x2, etc rather than offsets off of the new _MergedGlobal.<br>Basically those static local symbols no longer exist and all uses of<br>them in the program have been replaced.<br><br>-eric<br><br><br>-Jim<br><br><br>-Quentin<br><br>On Apr 16, 2013, at 5:31 PM, Jim Grosbach <<a href="mailto:grosbach@apple.com">grosbach@apple.com</a>> wrote:<br><br>Hi Quentin,<br><br>Hmmmm…. Perhaps we should consider whether to move the pass to where it was<br>actually really running before. We’ll want to consider whether the old<br>behavior was a bug, though, and it really belongs where it’s actually<br>running now (later). A quick glance suggests that the test case may just<br>need to be updated (i.e., the new order is fine, just different). We should<br>understand a bit more of what’s going on before assuming that, though.<br><br>-Jim<br><br>On Apr 16, 2013, at 4:27 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><br>Hi,<br><br>I’ve noticed that all the work performed by the GlobalMerge pass is done<br>within the doInitialization method.<br>It’s unclear to me if it is a valid design or not, because according to<br><a href="http://llvm.org/docs/WritingAnLLVMPass.html#the-functionpass-class:">http://llvm.org/docs/WritingAnLLVMPass.html#the-functionpass-class:</a><br>- On one hand:<br>FunctionPass subclasses are not allowed to:<br>[…]<br>3. Add or remove global variables from the current Module.<br>- On the other hand:<br>The doInitialization method is allowed to do most of the things that<br>FunctionPasses are not allowed to do.<br><br>What is clear however is: This design prevents other passes to take<br>advantage of this pass. Let’s say I write a pass A that creates some global<br>variables and I’d like to see these variables merged. Thus, I’ll add<br>GlobalMerge pass after A. This does not work unless I push all the work of<br>pass A into doInitialization…<br><br>The proposed patch turns GlobalMerge into a ModulePass and move all its work<br>in runOnModule method.<br>Although this patch does not intend to make any functionality change it<br>seems that the phase ordering implied by this design was expected!<br><br>Indeed, with this patch I have two make check failures related to debug<br>info:<br>test/CodeGen/ARM/2011-01-19-MergedGlobalDbg.ll:26:14: error: expected string<br>not found in input<br>;CHECK-NEXT: .long __MergedGlobals<br> ^<br><stdin>:233:2: note: scanning from here<br>.long _x2<br>^<br>--<br>and<br>test/CodeGen/ARM/2011-08-02-MergedGlobalDbg.ll:17:14: error: expected string<br>not found in input<br>;CHECK-NEXT: .long __MergedGlobals<br> ^<br><stdin>:205:2: note: scanning from here<br>.long _x2<br>^<br>—<br><br>Any ideas on how I can fix those?<br><br>-Quentin<br><GlobalMergeToModulePass.patch><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</blockquote></div></blockquote></div><br></body></html>