<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 10, 2015 at 9:48 AM, Diego Novillo <span dir="ltr"><<a href="mailto:dnovillo@google.com" target="_blank">dnovillo@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Apr 10, 2015 at 11:22 AM, Duncan P. N. Exon Smith<br>
<<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
<br>
> Why? They'll have different line table entries, even in -gmlt. Here's<br>
> what `llvm-dwarfdump -debug-dump=line` gives with my patch:<br>
<br>
</span>Because column information cannot be reliably used to distinguish<br>
multiple basic blocks on the same line.</blockquote><div><br>I don't really understand why line information is any more or less reliable than column information, though?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> In this example we are<br>
discussing, it can, but in other circumstances it is not possible.<br></blockquote><div><br>Certainly that's true - that's why we need discriminators, for when things aren't /otherwise/ disambiguated (eg: if they're on different lines they're different, that's easy - it's when it's two basic blocks on the same line that it gets hard - but if, again, they're in different columns that seems as good as when they're on different lines - and again, falling back to discriimnators when they happen to be at the same column as well)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The bigger issues are macro expansions and compiler generated builtin<br>
expansions (say, memcpy). There are other examples in the original<br>
dwarf proposal (<a href="http://wiki.dwarfstd.org/index.php?title=Path_Discriminators" target="_blank">http://wiki.dwarfstd.org/index.php?title=Path_Discriminators</a>).<br>
<br>
This is why, we don't even try to use columns. The logic in the sample<br>
profiler uses line number and discriminator exclusively because of the<br>
various limitations that column info has.</blockquote><div><br>That description doesn't seem to mention column info or its limitations... does it? (I just searched for column, the only mention I can find is describing the line table itself and the discriminator column, not column info)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> So, if two distinct basic<br>
blocks share the same line number, we make sure that they get<br>
different discriminator numbers.<br>
<span class=""><br>
> Address Line Column File ISA Discriminator Flags<br>
> ------------------ ------ ------ ------ --- ------------- -------------<br>
> 0x0000000000000000 2 0 1 0 0 is_stmt<br>
> 0x0000000000000007 4 9 1 0 0 is_stmt prologue_end<br>
> 0x000000000000000b 4 7 1 0 0<br>
> 0x0000000000000013 4 21 1 0 0<br>
> 0x0000000000000016 4 19 1 0 0<br>
> 0x0000000000000019 5 9 1 0 0 is_stmt<br>
> 0x000000000000001c 5 7 1 0 0<br>
> 0x000000000000001f 7 1 1 0 0 is_stmt<br>
> 0x0000000000000021 7 1 1 0 0 is_stmt end_sequence<br>
><br>
> 0xb is the end of the one basic block, and 0x13 is the start of the next.<br>
> I can easily tell them apart ;). Why do you need a discriminator? Without<br>
> my patch, AddDiscriminators would set the discriminator to `1` on the 0x13<br>
> entry. Why?<br>
<br>
</span>Because of the limitations with column information. The sample profile<br>
pass will not even try to use column numbers. If the code came from<br>
something like this:<br>
<br>
#define CMP_INC(x, y) if (x > 0) x++; else y--<br>
<br>
int foo(int x, int y) {<br>
CMP_INC(x,y);<br>
CMP_INC(y,x);<br>
}<br>
<br>
Even with column information, you get the two expansions of CMP_INC at column 3.<br>
<span class=""><br>
> E.g., your example in -g but without column info would give this with my<br>
> patch:<br>
><br>
> Address Line Column File ISA Discriminator Flags<br>
> ------------------ ------ ------ ------ --- ------------- -------------<br>
> 0x0000000000000000 2 0 1 0 0 is_stmt<br>
> 0x0000000000000007 4 0 1 0 0 is_stmt prologue_end<br>
> 0x000000000000000b 4 0 1 0 0<br>
> 0x0000000000000013 4 0 1 0 0<br>
> 0x0000000000000019 5 0 1 0 0 is_stmt<br>
> 0x000000000000001f 7 0 1 0 0 is_stmt<br>
> 0x0000000000000021 7 0 1 0 0 is_stmt end_sequence<br>
><br>
> Do you need a discriminator here, even though there are separate<br>
> entries? Why?<br>
<br>
</span>Not in this case. The problem is that we cannot always reliably use<br>
this entry equivalence. In some cases (even with column info on),<br>
entry equivalence will be the wrong test. There will be two distinct<br>
basic blocks with the exact same LOC entry.<br>
<br>
Yeah, it's kind of irritating.<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
Diego.<br>
</font></span></blockquote></div><br></div></div>