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