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.
Jakub
