Re: [PATCH] Update source location for PRE inserted stmt

2012-11-26 Thread Dehao Chen
On Sun, Nov 25, 2012 at 11:37 AM, Xinliang David Li wrote: > On Sun, Nov 25, 2012 at 4:40 AM, Richard Biener > wrote: >> On Thu, Nov 15, 2012 at 5:46 PM, Eric Botcazou wrote: But UNKNOWN_LOCATION is effectively wrong as well. If other optimizations move the statements above the insert

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-25 Thread Xinliang David Li
On Sun, Nov 25, 2012 at 4:40 AM, Richard Biener wrote: > On Thu, Nov 15, 2012 at 5:46 PM, Eric Botcazou 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 wha

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-25 Thread Richard Biener
On Thu, Nov 15, 2012 at 5:46 PM, Eric Botcazou 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. >

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-15 Thread Dehao Chen
Yeah, at least for the unittest I provided, the coverage info will be wrong without the patch. Thanks, Dehao On Thu, Nov 15, 2012 at 10:30 AM, Xinliang David Li wrote: > I probably made too general statement in this topic. However for the > PRE case, I believe the choice of not using UNKNOWN loc

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-15 Thread Xinliang David Li
I probably made too general statement in this topic. However for the PRE case, I believe the choice of not using UNKNOWN location is still better. thanks, David On Thu, Nov 15, 2012 at 9:23 AM, Eric Botcazou wrote: >> The randomness here means that if we set UNKNOWN_LOCATION to insn, it >> can

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-15 Thread Eric Botcazou
> The randomness here means that if we set UNKNOWN_LOCATION to insn, it > can get source location anywhere. Even with some small code layout > changes, the location for that insn could change. I would hope that in > the future, we add an assertion when emitting instruction to enforce > that INSN_LO

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-15 Thread Dehao Chen
On Thu, Nov 15, 2012 at 8:46 AM, Eric Botcazou 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 stateme

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-15 Thread Eric Botcazou
> 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, alt

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-15 Thread Diego Novillo
On 2012-11-05 06:54 , Eric Botcazou wrote: Those compiler generated statements do not have source origins, but they need to have debug location information attached so that information collected for them can be correlated with program constructs (such as CFG). One of the natural way is to use the

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-05 Thread Xinliang David Li
For the example I listed, the new statement is generated for source construct at program point (2). However unlike simple code motion, (2) is not going away after PRE. How would setting the location of the new statement at the insertion point break coverage? Besides, the new statement won't create

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-05 Thread Dehao Chen
> No, there is nothing natural (and this can even be wrong). The statements > must have the source location corresponding to the source construct they are > generated for, which may be totally different from that of the insertion > point. Yes, that can generate jumpiness in GDB, but this is far b

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-05 Thread Eric Botcazou
> Those compiler generated statements do not have source origins, but > they need to have debug location information attached so that > information collected for them can be correlated with program > constructs (such as CFG). One of the natural way is to use the source > location associated with th

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-04 Thread Xinliang David Li
On Sun, Nov 4, 2012 at 8:07 AM, Richard Biener wrote: > On Wed, Oct 31, 2012 at 8:02 PM, Xinliang David Li wrote: >> Dehao's patch will make the debugging of the following code (-g -O2) >> less jumpy. After the testing of x > 0, it should go to line 'a++'. >> Without the fix, when stepping thr

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-04 Thread Dehao Chen
2. __attribute__((noinline)) int abc (int *a) 3. { 4. int ret = 0; 5. 6. if (x > 0) 7.ret += *a; 8. else 9.a++; 10. 11. ret += *a; 12. return ret; 13 } In terms of jumpiness, without the patch, the jump sequence is: 2 -> 13 -> 4 -> 11 -> 13 With the patch, the jump sequence is: 2-> 9

Re: [PATCH] Update source location for PRE inserted stmt

2012-11-04 Thread Richard Biener
On Wed, Oct 31, 2012 at 8:02 PM, Xinliang David Li wrote: > Dehao's patch will make the debugging of the following code (-g -O2) > less jumpy. After the testing of x > 0, it should go to line 'a++'. > Without the fix, when stepping through 'abc', the lines covered are > 6, 4, 11, 13. With the f

Re: [PATCH] Update source location for PRE inserted stmt

2012-10-31 Thread Xinliang David Li
Dehao's patch will make the debugging of the following code (-g -O2) less jumpy. After the testing of x > 0, it should go to line 'a++'. Without the fix, when stepping through 'abc', the lines covered are 6, 4, 11, 13. With the fix it should be 6, 9, 11, 13 -- much better. David 1. int x;

Re: [PATCH] Update source location for PRE inserted stmt

2012-10-31 Thread Dehao Chen
On Wed, Oct 31, 2012 at 2:34 AM, Richard Biener wrote: > On Wed, Oct 31, 2012 at 12:57 AM, Xinliang David Li > wrote: >> It will make the location info for the newly synthesized stmt more >> deterministic, I think. > > Maybe, but it will increase the jumpiness in the debugger without actually >

Re: [PATCH] Update source location for PRE inserted stmt

2012-10-31 Thread Richard Biener
On Wed, Oct 31, 2012 at 12:57 AM, Xinliang David Li wrote: > It will make the location info for the newly synthesized stmt more > deterministic, I think. Maybe, but it will increase the jumpiness in the debugger without actually being accurate, no? For example if the partially redundant expressi

Re: [PATCH] Update source location for PRE inserted stmt

2012-10-30 Thread Xinliang David Li
It will make the location info for the newly synthesized stmt more deterministic, I think. David On Tue, Oct 30, 2012 at 4:38 PM, Steven Bosscher wrote: > On Wed, Oct 31, 2012 at 12:00 AM, Dehao Chen wrote: >> This patch aims to improve debugging of optimized code. It ensures >> that PRE inserte

Re: [PATCH] Update source location for PRE inserted stmt

2012-10-30 Thread Dehao Chen
Sorry, new patch attached... On Tue, Oct 30, 2012 at 4:38 PM, Steven Bosscher wrote: > On Wed, Oct 31, 2012 at 12:00 AM, Dehao Chen wrote: >> This patch aims to improve debugging of optimized code. It ensures >> that PRE inserted statements have the same source location as the >> statement at the

Re: [PATCH] Update source location for PRE inserted stmt

2012-10-30 Thread Steven Bosscher
On Wed, Oct 31, 2012 at 12:00 AM, Dehao Chen wrote: > This patch aims to improve debugging of optimized code. It ensures > that PRE inserted statements have the same source location as the > statement at the insertion point, instead of UNKNOWN_LOCATION. Wrong patch attached. However, is it really

[PATCH] Update source location for PRE inserted stmt

2012-10-30 Thread Dehao Chen
Hi, This patch aims to improve debugging of optimized code. It ensures that PRE inserted statements have the same source location as the statement at the insertion point, instead of UNKNOWN_LOCATION. Bootstrapped on x86_64, and passed gcc regression tests and gdb regression tests. Is it okay for