On Sun, Nov 25, 2012 at 4:40 AM, Richard Biener <richard.guent...@gmail.com> wrote: > On Thu, Nov 15, 2012 at 5:46 PM, Eric Botcazou <ebotca...@adacore.com> wrote: >>> But UNKNOWN_LOCATION is effectively wrong as well. If other >>> optimizations move the statements above the inserted instruction, then >>> the new instruction ends up inheriting whatever location happens to be >>> in the previous statement. >> >> That's correct, UNKNOWN_LOCATION isn't a panacea either, although it can help >> in some cases. The only safe thing to do is to put the exact source location >> of the statement, i.e. the location of the source construct for which the >> statement has been generated. > > There may not be a source location for a generated statement, the computed > value might not have been computed in the source at any point even. Consider > re-association of an expression and then a re-associated part becoming > partially redundant. > > if () > tem = a + b; > tem2 = a + c + b; > > after re-assoc + PRE: > > if () > tem = a + b; > else > tem' = a + b; // which sloc here?
If there was a else branch, the proposed patch by Dehao will use the source location of the previous stmt 'tem' = a + b' was inserted after. I did not see any special source location handling in tree-ssa-reassoc.c -- looks like the same principle is used there -- the new assignment after the operand shuffling will inherit the source location of the placeholder gimple stmt. > tem'' = PHI <tem, tem'> // or here? on the args? > tem2 = tem'' + c; > > so what source location would you use for the inserted expression? If > UNKNOWN is not > correct here then I think we need an explicit NOWHERE? Can that break line coverage info too, say, when source location get stripped from reassociated expressions? David > >>> Fixing the location when the statement is inserted will reduce this >>> randomness. I'm not sure I understand why you think this will break >>> coverage. The examples given in this thread were helped by this >>> location fixing. >> >> What do you call randomness exactly? GDB jumpiness? I'm a little wary of >> fixing GDB jumpiness by distorting the debug info. Ian has clearly outlined >> the correct approach to addressing it. >> >> Note that I didn't specifically reply to Dehao's patch here, which might be >> acceptable in the end, only to David's seemingly general statement about the >> "natural way" of putting a location on these statements. > > Richard. > >> -- >> Eric Botcazou