On Tue, Oct 30, 2012 at 4:03 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Tue, Oct 30, 2012 at 03:38:11PM +0100, Richard Biener wrote:
>> I question the need of BLOCK info on expression trees. If BLOCKs are
>> relevant then the tree ends up referencing a declaration with a BLOCK as
>> context, no? Thus, the case
>>
>> int tem, a;
>> {
>> int a;
>> ...
>> tem = a;
>> }
>> int b = tem + 5;
>>
>> where we may end up with gimple like
>>
>> b = a + 5;
>>
>> thus mixing two BLOCKs inside a stmt (and no expression trees to
>> attach different BLOCKs) is no different from the case where we end
>> up with expression trees.
>
> IMHO either we don't use locations at all on tree expressions (thus
> no source location nor block), or both. A source location has always
> an associated block it is present in. Of course for shared trees we
> can't put there any block, as it can appear anywhere.
>
>> Thus my original question - why isn't a NULL BLOCK treated the same
>> as UNKNOWN_LOCATION? Or rather, _why_ does Michas patch not work?
>> Did you analyze the guality fails?
>
> Micha's patch is degrading debug info quality. Whenever some expression
> has different source location from the source location of the gimple stmt,
> then assuming that other source location is from the same block is wrong,
> it could very well be from a different one. On the testcase that fails
> with Micha's patch, we have:
> [pr43479.c : 8:4] l_2 = l_1(D) + 1;
> [pr43479.c : 8:4] # DEBUG l => l_2
> [pr43479.c : 10:9] # DEBUG h => n_3(D)
> [pr43479.c : 12:11] # DEBUG i => k_4(D)
> [pr43479.c : 13:8] k_5 = k_4(D) + 1;
> [pr43479.c : 13:8] # DEBUG k => k_5
> [pr43479.c : 17:11] # DEBUG j => m_6(D)
> [pr43479.c : 18:8] m_7 = m_6(D) + 1;
> [pr43479.c : 18:8] # DEBUG m => m_7
> [pr43479.c : 22:3] __asm__ __volatile__("" : : "r" k_5, "r" l_2);
> [pr43479.c : 23:3] __asm__ __volatile__("" : : "r" m_7, "r" n_3(D));
> where line 8 is from the outer block in the source, line 10 from the middle
> block, line 12/13 from first innermost block, line 17/18 from second
> innermost block. But all of the l_2, k_5 and m_7 setters are TERed,
> so everything is emitted when expanding the two asm
> statements and with Micha's patch the block used is the block of the asm
> statement, while previously each TERed statement got its own block.
>
> I'd say either we should do the TREE_BLOCK setting on all non-shareable
> trees during gimple-low and clear the block (but then likely whole
> location?; it doesn't make sense to say in the debugger that something
> has certain source location when you can't print variables declared in that
> location) if copying expressions for use elsewhere, outside of the
> containing function. Or say during gimplification or gimple-low.c
> simply set t->exp.locus of all non-shareable expressions to UNKNOWN_LOCATION
> to make it clear we don't use it (wonder if that could affect debug info
> quality, perhaps not that much), but during expansion if creating trees
> from TERed stmts they need to be set back, or the current location/block
> adjusted accordingly.
So maybe TER (well, those looking up the stmt) should pick the location
from the TERed statement properly then?
Richard.
> Jakub
