<div dir="ltr">Great, please commit!</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 3, 2015 at 10:29 AM, Dario Domizioli <span dir="ltr"><<a href="mailto:dario.domizioli@gmail.com" target="_blank">dario.domizioli@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Rebased patch with added comment and without test.<span class=""><div><br></div><div><br></div><div>Cheers,</div><div> <span style="color:rgb(80,0,80);font-size:12.8000001907349px">Dario Domizioli</span></div><div style="color:rgb(80,0,80);font-size:12.8000001907349px"> SN Systems - Sony Computer Entertainment Group</div><div><br></div><div><br></div><div><br></div><div><br></div></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On 3 March 2015 at 18:22, Dario Domizioli <span dir="ltr"><<a href="mailto:dario.domizioli@gmail.com" target="_blank">dario.domizioli@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thanks David!<div><br></div><div>The test case is actually pretty reliable (I'm surprised myself). Five scopes and twenty-five declarations seem to be enough to trigger the unstable sort behaviour, at least with the std::sort in glibc.</div><div>Still, it is a maintenance burden if the metadata format changes, so I defer to your judgement.</div><div><br></div><div>Would a comment like this be OK?</div><div><br></div><div><div> // Stable sort to preserve the order of appearance of imported entities.</div><div> // This is to avoid out-of-order processing of interdependent declarations</div><div> // within the same scope, e.g. { namespace A = base; namespace B = A; }</div></div><div><br></div><div>(I'm rebasing the patch and rebuilding at the moment)</div><span><div><br></div><div>Cheers,</div><div> Dario Domizioli</div><div> SN Systems - Sony Computer Entertainment Group</div><div><br></div><div><br></div><div><br></div></span></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On 3 March 2015 at 17:33, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I think this sounds reasonable - though, yeah, it is perhaps a bit subtle that the order of entries in the imported entities list must be such that there are no forward references. I'm kind of OK with that.<br><br>A comment on the sort explaining why it needs to be stable wouldn't go astray.<br><br>& I'd probably skip the test case - testing for non-determinism is a bit of a lost cause. We regularly fix non-determinism bugs without corresponding test cases.</div><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Tue, Mar 3, 2015 at 8:13 AM, Dario Domizioli <span dir="ltr"><<a href="mailto:dario.domizioli@gmail.com" target="_blank">dario.domizioli@gmail.com</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div dir="ltr">Hello LLVM,<div><br></div><div>I have found and filed PR22750, and I've looked into fixing it.</div><div><br></div><div>Basically, it's an assertion failure during debug info generation, and its cause is the interaction of two factors:</div><div>1) When generating a DW_TAG_imported_declaration DIE which imports another imported declaration, the code in lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp asserts that the second imported declaration must already have a DIE (line 661).<br></div><div>2) There is a non-determinism in the order in which imported declarations within the same scope are processed.</div><div><br></div><div>Because of the non-determinism (2), it is possible that an imported declaration is processed before another one it depends on, breaking the assumption in (1).</div><div><br></div><div>I have tracked down the source of the non-determinism.</div><div>The imported declaration DIDescriptors are sorted by scope in DwarfDebug::beginModule(), in lib/CodeGen/AsmPrinter/DwarfDebug.cpp:480; however that sort is not a stable_sort, therefore the order of the declarations within the same scope is not preserved.</div><div><br></div><div>The attached patch changes the std::sort to a std::stable_sort and it fixes the problem.</div><div><br></div><div>I'm not 100% sure this is the right solution, even if it works. My question is: would it be better to relax the assumption in (1), e.g. by making the code create the dependency DIE on demand?</div><div><br></div><div>Cheers,</div><div> Dario Domizioli</div><div> SN Systems - Sony Computer Entertainment Group</div><div><br></div><div><br></div></div>
<br></div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">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>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>