[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 --- Comment #14 from Dmitry G. Dyachenko --- (In reply to rguent...@suse.de from comment #13) > On Fri, 18 Oct 2019, dimhen at gmail dot com wrote: > I guess > previously the uninit pass didn't emit warnings for the load > because it had no location. So new warnings are expected. Nice! Thank you
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929
--- Comment #13 from rguenther at suse dot de ---
On Fri, 18 Oct 2019, dimhen at gmail dot com wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929
>
> Dmitry G. Dyachenko changed:
>
>What|Removed |Added
>
> CC||dimhen at gmail dot com
>
> --- Comment #12 from Dmitry G. Dyachenko ---
> I see new warnings -Wuninitialized and -Wmaybe-uninitialized after r276993
>
> r276992 no warnings
> r276993 warnings
>
> $ cat x_3.i
> int *a;
> int b, d;
>
> int g() {
> int *c;
> int e[6];
> int f = 1;
> if (0)
> goto cd;
> c = 0;
> for (; d; d++)
> *e = 1 ^ *(c + 1);
> if (f)
> for (b = 0;;)
> a[0] = e[b];
> cd:
> return 0;
> }
>
> $ ~/arch-gcc/gcc_276993/bin/gcc -fpreprocessed -O2 -Wall -c x_3.i
>
> x_3.i: In function ‘g’:
> x_3.i:15:15: warning: ‘e[0]’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>15 | a[0] = e[b];
> | ~^~~
If d is zero then e[0] will be loaded uninitialized. PRE exposes
this to the uninit machinery which previously gave up for loops
by commoning e[b] with the set in the loop plus inserting
a load from e[0] on the path not coming from the loop. I guess
previously the uninit pass didn't emit warnings for the load
because it had no location.
> $ cat x.i
> typedef struct {
> int a[0];
> } c;
> typedef struct {
> c d;
> } * e;
> e a;
> void f(void);
> void f() {
> int c[1];
> for (;;) {
> unsigned long d[0];
> int b, g, h = b = h;
> unsigned long *e = d;
> for (; g; ++g)
> e[g] = 0;
> *a->d.a = *c;
> }
> }
>
> $ ~/arch-gcc/gcc_276993/bin/gcc -fpreprocessed -O2 -Wall -c x.i
> x.i: In function ‘f’:
> x.i:17:13: warning: ‘c[0]’ is used uninitialized in this function
> [-Wuninitialized]
>17 | *a->d.a = *c;
> | ^~~~
This is the same (and g is not initialized here as well)
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929
Dmitry G. Dyachenko changed:
What|Removed |Added
CC||dimhen at gmail dot com
--- Comment #12 from Dmitry G. Dyachenko ---
I see new warnings -Wuninitialized and -Wmaybe-uninitialized after r276993
r276992 no warnings
r276993 warnings
$ cat x_3.i
int *a;
int b, d;
int g() {
int *c;
int e[6];
int f = 1;
if (0)
goto cd;
c = 0;
for (; d; d++)
*e = 1 ^ *(c + 1);
if (f)
for (b = 0;;)
a[0] = e[b];
cd:
return 0;
}
$ ~/arch-gcc/gcc_276993/bin/gcc -fpreprocessed -O2 -Wall -c x_3.i
x_3.i: In function ‘g’:
x_3.i:15:15: warning: ‘e[0]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
15 | a[0] = e[b];
| ~^~~
$ cat x.i
typedef struct {
int a[0];
} c;
typedef struct {
c d;
} * e;
e a;
void f(void);
void f() {
int c[1];
for (;;) {
unsigned long d[0];
int b, g, h = b = h;
unsigned long *e = d;
for (; g; ++g)
e[g] = 0;
*a->d.a = *c;
}
}
$ ~/arch-gcc/gcc_276993/bin/gcc -fpreprocessed -O2 -Wall -c x.i
x.i: In function ‘f’:
x.i:17:13: warning: ‘c[0]’ is used uninitialized in this function
[-Wuninitialized]
17 | *a->d.a = *c;
| ^~~~
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 Richard Biener changed: What|Removed |Added Status|ASSIGNED|RESOLVED Known to work||10.0 Resolution|--- |FIXED --- Comment #11 from Richard Biener --- Fixed on trunk.
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 --- Comment #10 from Richard Biener --- Author: rguenth Date: Tue Oct 15 11:47:27 2019 New Revision: 276993 URL: https://gcc.gnu.org/viewcvs?rev=276993&root=gcc&view=rev Log: 2019-10-15 Richard Biener PR tree-optimization/91929 * tree-ssa-pre.c (pre_expr_d::loc): New member. (get_or_alloc_expr_for_name): Initialize it. (get_or_alloc_expr_for_constant): Likewise. (phi_translate_1): Copy it. (create_expression_by_pieces): Use the original location of the expression for the inserted stmt. (compute_avail): Record the location of the stmt for the expressions created. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-ssa-pre.c
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 --- Comment #9 from Milian Wolff --- > And I'm not sure that the original behavior which for > this particular case would simply say sin() was called from foo() This would indeed be the best, but that didn't happen originally when `foo` itself got inlined like in my example. See again the original backtrace from GDB: ``` Breakpoint 2, 0x00418790 in __cos_fma () (gdb) bt #0 0x00418790 in __cos_fma () #1 0x00401573 in std::generate_n >, int, main():: > (__n=10, __gen=..., __first=...) at /usr/include/c++/9.1.0/new:174 #2 main () at ../../../manual/clients/vector.cpp:16 ``` This is very confusing to the end user, and personally I think it would be better to have at least one wrong branch here rather than none at all.
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929
--- Comment #8 from Richard Biener ---
(In reply to Milian Wolff from comment #7)
> to me, that backtrace looks quite nice and usable - a huge improvement,
> thanks!
>
> what you are saying is that if the same file would be calling sin/cos
> somewhere else, only one of those inline locations would show up? too bad,
> but I can see how it's impossible to map this in an efficient way...
Yeah, so consider
double A (double x) { return sin(x); }
double B (double x) { return sin(x); }
double foo(double x, int which)
{
double res;
if (which == 1)
res = A (x);
else
res = B (x);
}
when GCC inlines both functions and performs code hoisting to get the
following optimized function then the call to sin will always appear
to come from either A or B (and that quite randomly).
double foo(double x)
{
return sin(x);
}
This exact situation of course shouldn't happen very often but with C++
and some more contrieved examples you may run into a situation that can
be mapped to this. And I'm not sure that the original behavior which
for this particular case would simply say sin() was called from foo()
wouldn't be better than the patched behavior which says the call
was always from A.
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 --- Comment #7 from Milian Wolff --- to me, that backtrace looks quite nice and usable - a huge improvement, thanks! what you are saying is that if the same file would be calling sin/cos somewhere else, only one of those inline locations would show up? too bad, but I can see how it's impossible to map this in an efficient way...
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 Richard Biener changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2019-10-14 Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #6 from Richard Biener --- Created attachment 47029 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47029&action=edit patch for PRE So GCC emits .L40: .loc 12 4459 13 is_stmt 0 view .LVU116 pxor%xmm0, %xmm0 addl$3, %ebx movq48(%rsp), %r13 cvtsi2sdl %r12d, %xmm0 callsin .LVL39: movsd %xmm0, 8(%rsp) pxor%xmm0, %xmm0 cvtsi2sdl %ebx, %xmm0 movl%r12d, %ebx callcos .LVL40: movsd 8(%rsp), %xmm2 movapd %xmm0, %xmm1 movapd %xmm2, %xmm0 callcabs .LVL41: .L23: .LBB577: .LBI577: .loc 9 12 52 is_stmt 1 view .LVU117 where the surrounding locations are not exactly helpful (9 refers to t.C while 12 refers to bits/stl_algo.h), but matches your observation. GCC-wise I don't see a very good way to address this in an "exact" manner. We'd somehow have to tell the debug consumer the stmts are from two (multiple in general) different locations at the same time. Alternatively we could pick one but the way PRE is implemented this could mean jumping between one of the two (or many) alternatives for each stmt since consistency here cannot be guaranteed. The attached achieves that, it ends up accumulating the locations to the end of the lambda. (gdb) bt #0 __cos_avx (x=3) at ../sysdeps/ieee754/dbl-64/s_sin.c:267 #1 0x00400862 in std::cos (__x=) at /home/space/rguenther/install/gcc-9.2/include/c++/9.2.0/bits/stl_algo.h:4459 #2 operator() (__closure=) at t.C:14 #3 std::generate_n >, int, main():: > (__n=10, __gen=..., __first=...) at /home/space/rguenther/install/gcc-9.2/include/c++/9.2.0/bits/stl_algo.h:4460 #4 main () at t.C:16
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 --- Comment #5 from Milian Wolff --- > Note the line number program should have picked up a location from the surrounding code, at least the surrounding function, so the ?? in the backtraces look like a consumer (perf) issue to me. All major DWARF consumers get it wrong, no? addr2line from binutils / libbfd, elfutils / libdwfl and also GDB - none of them find any information for __cos_fma.
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 Richard Biener changed: What|Removed |Added CC||rguenth at gcc dot gnu.org --- Comment #4 from Richard Biener --- (In reply to Andrew Pinski from comment #3) > PRE is removing the line numbers for some reason. PRE creates expressions for values out of thin air - there isn't a "correct" line number it can use. So we end up with the following after PRE insertion: pre_inserted = sin (x); // no line number if (...) original1 = sin (x); original2 = sin (x); at the time we perform CSE on this we could in theory attach line numbers to pre_inserted when replacing a sin (x) call with pre_inserted. But we have two - so which one do we choose? Note the line number program should have picked up a location from the surrounding code, at least the surrounding function, so the ?? in the backtraces look like a consumer (perf) issue to me. Yes, the sin() call may fall out of an inlined function context or even end up in the wrong one if you consider inlining two different functions calling sin on the same value. But again I don't see a good way to "fix" this.
[Bug debug/91929] missing inline subroutine information in build using sin/cos
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91929 Andrew Pinski changed: What|Removed |Added Summary|missing inline subroutine |missing inline subroutine |information in static build |information in build using |using sin/cos |sin/cos --- Comment #3 from Andrew Pinski --- PRE is removing the line numbers for some reason.
