On 10/23/14 01:30, Jakub Jelinek wrote:
Hi!As I wrote in the PR, vt_stack_adjustments can often compute wrong offsets, because it never considers pops with autoinc addressing, which can lead either to wrong debug info, or turning off -fvar-tracking altogether for a function on which that issue resulted in stack depth inconsistencies on the edges. Here are some stats from --enable-checking=yes,rtl cc1plus bootstrapped with/without the patch (without the patch got then rebuilt stage3 with the patch, i.e. just var-tracking.o and cc1plus-checksum.o got recompiled, so I'm comparing identical code, different debug info): x86_64-linux cc1plus built without the patch: cov% samples cumul 0.0 506230/38% 506230/38% 0..10 10327/0% 516557/39% 11..20 12390/0% 528947/39% 21..30 31265/2% 560212/42% 31..40 18775/1% 578987/43% 41..50 20631/1% 599618/45% 51..60 24921/1% 624539/47% 61..70 40959/3% 665498/50% 71..80 23771/1% 689269/52% 81..90 41771/3% 731040/55% 91..99 81667/6% 812707/61% 100 510564/38% 1323271/100% x86_64-linux cc1plus built with the patch: cov% samples cumul 0.0 382214/28% 382214/28% 0..10 13100/0% 395314/29% 11..20 14568/1% 409882/30% 21..30 33708/2% 443590/33% 31..40 21927/1% 465517/35% 41..50 23924/1% 489441/36% 51..60 28736/2% 518177/39% 61..70 45847/3% 564024/42% 71..80 29284/2% 593308/44% 81..90 52085/3% 645393/48% 91..99 99971/7% 745364/56% 100 577907/43% 1323271/100% i686-linux cc1plus built without the patch: cov% samples cumul 0.0 631348/48% 631348/48% 0..10 7764/0% 639112/48% 11..20 9690/0% 648802/49% 21..30 25036/1% 673838/51% 31..40 16113/1% 689951/52% 41..50 19753/1% 709704/54% 51..60 14563/1% 724267/55% 61..70 34093/2% 758360/58% 71..80 17450/1% 775810/59% 81..90 31339/2% 807149/61% 91..99 60368/4% 867517/66% 100 437548/33% 1305065/100% i686-linux cc1plus built with the patch: cov% samples cumul 0.0 377352/28% 377352/28% 0..10 16077/1% 393429/30% 11..20 15390/1% 408819/31% 21..30 31790/2% 440609/33% 31..40 23889/1% 464498/35% 41..50 29267/2% 493765/37% 51..60 22902/1% 516667/39% 61..70 45629/3% 562296/43% 71..80 29511/2% 591807/45% 81..90 50536/3% 642343/49% 91..99 93584/7% 735927/56% 100 569138/43% 1305065/100% .debug_info/.debug_loc sizes in bytes: x86_64-linux cc1plus without patch .debug_info 75411710, .debug_loc 75421077 x86_64-linux cc1plus with patch .debug_info 78498790, .debug_loc 90530117 i686-linux cc1plus without patch .debug_info 59921183, .debug_loc 37823166 i686-linux cc1plus with patch .debug_info 63009554, .debug_loc 59535100 I've also performed instrumented bootstraps/regtests (x86_64-linux and i686-linux), where I've logged in how many functions the result of vt_stack_adjustments differed between the bad old way and new way. In both the bootstraps/regtests, it affected 16892 32-bit and 6646 64-bit functions, in all cases it was old way giving up and new way succeeding. Not adding a testcase, as the one in the PR failed to produce proper debug info only in 4.8 (then got latent), there already are some guality improvements with the patch: -FAIL: gcc.dg/guality/pr54693-2.c -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions line 21 i == v + 1 -FAIL: gcc.dg/guality/pr54693-2.c -O3 -fomit-frame-pointer -funroll-loops line 21 i == v + 1 on x86_64 and: -FAIL: gcc.dg/guality/pr54519-1.c -O2 line 20 y == 25 -FAIL: gcc.dg/guality/pr54519-1.c -O2 line 20 z == 6 -FAIL: gcc.dg/guality/pr54519-1.c -O2 line 23 y == 117 -FAIL: gcc.dg/guality/pr54519-1.c -O2 line 23 z == 8 -FAIL: gcc.dg/guality/pr54519-1.c -O3 -fomit-frame-pointer line 20 x == 36 -FAIL: gcc.dg/guality/pr54519-1.c -O3 -fomit-frame-pointer line 20 y == 25 -FAIL: gcc.dg/guality/pr54519-1.c -O3 -fomit-frame-pointer line 20 z == 6 -FAIL: gcc.dg/guality/pr54519-1.c -O3 -g line 20 x == 36 -FAIL: gcc.dg/guality/pr54519-1.c -O3 -g line 20 y == 25 -FAIL: gcc.dg/guality/pr54519-1.c -O3 -g line 20 z == 6 -FAIL: gcc.dg/guality/pr54519-3.c -O2 line 20 x == 36 -FAIL: gcc.dg/guality/pr54519-3.c -O2 line 20 y == 25 -FAIL: gcc.dg/guality/pr54519-3.c -O2 line 20 z == 6 -FAIL: gcc.dg/guality/pr54519-3.c -O2 line 23 x == 98 -FAIL: gcc.dg/guality/pr54519-3.c -O2 line 23 y == 117 -FAIL: gcc.dg/guality/pr54519-3.c -O2 line 23 z == 8 -FAIL: gcc.dg/guality/pr54519-3.c -O2 -flto line 20 x == 36 -FAIL: gcc.dg/guality/pr54519-3.c -O2 -flto line 23 x == 98 -FAIL: gcc.dg/guality/pr54519-3.c -O2 -flto -flto-partition=none line 20 x == 36 -FAIL: gcc.dg/guality/pr54519-3.c -O2 -flto -flto-partition=none line 23 x == 98 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -fomit-frame-pointer line 20 x == 36 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -fomit-frame-pointer line 20 y == 25 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -fomit-frame-pointer line 20 z == 6 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -fomit-frame-pointer line 23 x == 98 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -fomit-frame-pointer line 23 y == 117 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -fomit-frame-pointer line 23 z == 8 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -g line 20 x == 36 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -g line 20 y == 25 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -g line 20 z == 6 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -g line 23 x == 98 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -g line 23 y == 117 -FAIL: gcc.dg/guality/pr54519-3.c -O3 -g line 23 z == 8 -FAIL: gcc.dg/guality/pr54693-2.c -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions line 21 i == v + 1 -FAIL: gcc.dg/guality/pr54693-2.c -O3 -fomit-frame-pointer -funroll-loops line 21 i == v + 1 on i686. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Not sure about branches, to some extent this must be a serious debug info quality regression, since the problem got significantly worse with omitting frame pointer by default on i686, and/or shrink wrapping and simple_return changes where there are more pops in the middle of functions. So with a short patch we could significantly improve debug info. The risk is compile time regressions, if we gave up on var-tracking, we threw away all the hard tracked debug stmts/debug insns, but don't need to costly propagate the values through the dataflow. 2014-10-23 Jakub Jelinek <ja...@redhat.com> PR debug/63623 * var-tracking.c (stack_adjust_offset_pre_post_cb): New function. (stack_adjust_offset_pre_post): Use it through for_each_inc_dec, instead of only handling autoinc in dest if it is a MEM. (vt_stack_adjustments): Fix up formatting.
OK. Jeff
