[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 Martin Jambor changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #15 from Martin Jambor --- Fixed (with the caveat explained in comment #10) on master and all opened release branches, thus closing.
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 --- Comment #14 from CVS Commits --- The releases/gcc-9 branch has been updated by Martin Jambor : https://gcc.gnu.org/g:7dd5b92bbe0944dc27e6175b0df72ed0a7188016 commit r9-9852-g7dd5b92bbe0944dc27e6175b0df72ed0a7188016 Author: Martin Jambor Date: Wed Dec 1 18:29:50 2021 +0100 ipa-sra: Check also ECF_LOOPING_CONST_OR_PURE when evaluating calls in PR 103267 Honza found out that IPA-SRA does not look at ECF_LOOPING_CONST_OR_PURE when evaluating if a call can have side effects. Fixed with this patch. The testcase infinitely loops in a const function, so it would not make a good addition to the testsuite. This patch is a manual backport of commit e5440bc08e07fd491dcccd47e1b86a5985ee117c to the old "early" IPA-SRA. gcc/ChangeLog: 2021-12-01 Martin Jambor PR ipa/103267 * tree-sra.c (scan_function): Also check ECF_LOOPING_CONST_OR_PURE flag.
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 --- Comment #13 from CVS Commits --- The releases/gcc-10 branch has been updated by Martin Jambor : https://gcc.gnu.org/g:d2ecc42195e8af1992d12e678e761c73557eaf56 commit r10-10318-gd2ecc42195e8af1992d12e678e761c73557eaf56 Author: Martin Jambor Date: Wed Dec 1 14:25:16 2021 +0100 ipa-sra: Check also ECF_LOOPING_CONST_OR_PURE when evaluating calls in PR 103267 Honza found out that IPA-SRA does not look at ECF_LOOPING_CONST_OR_PURE when evaluating if a call can have side effects. Fixed with this patch. The testcase infinitely loops in a const function, so it would not make a good addition to the testsuite. gcc/ChangeLog: 2021-11-29 Martin Jambor PR ipa/103267 * ipa-sra.c (scan_function): Also check ECF_LOOPING_CONST_OR_PURE flag. (cherry picked from commit e5440bc08e07fd491dcccd47e1b86a5985ee117c)
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 --- Comment #12 from CVS Commits --- The releases/gcc-11 branch has been updated by Martin Jambor : https://gcc.gnu.org/g:5e2e6cc84c46ff7ceb6395c0309a2c6b71d83cb1 commit r11-9347-g5e2e6cc84c46ff7ceb6395c0309a2c6b71d83cb1 Author: Martin Jambor Date: Wed Dec 1 14:13:35 2021 +0100 ipa-sra: Check also ECF_LOOPING_CONST_OR_PURE when evaluating calls in PR 103267 Honza found out that IPA-SRA does not look at ECF_LOOPING_CONST_OR_PURE when evaluating if a call can have side effects. Fixed with this patch. The testcase infinitely loops in a const function, so it would not make a good addition to the testsuite. gcc/ChangeLog: 2021-11-29 Martin Jambor PR ipa/103267 * ipa-sra.c (scan_function): Also check ECF_LOOPING_CONST_OR_PURE flag. (cherry picked from commit e5440bc08e07fd491dcccd47e1b86a5985ee117c)
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 --- Comment #11 from CVS Commits --- The master branch has been updated by Martin Jambor : https://gcc.gnu.org/g:e5440bc08e07fd491dcccd47e1b86a5985ee117c commit r12-5633-ge5440bc08e07fd491dcccd47e1b86a5985ee117c Author: Martin Jambor Date: Tue Nov 30 18:45:11 2021 +0100 ipa-sra: Check also ECF_LOOPING_CONST_OR_PURE when evaluating calls in PR 103267 Honza found out that IPA-SRA does not look at ECF_LOOPING_CONST_OR_PURE when evaluating if a call can have side effects. Fixed with this patch. The testcase infinitely loops in a const function, so it would not make a good addition to the testsuite. gcc/ChangeLog: 2021-11-29 Martin Jambor PR ipa/103267 * ipa-sra.c (scan_function): Also check ECF_LOOPING_CONST_OR_PURE flag.
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 --- Comment #10 from Martin Jambor --- I have proposed a patch to address this issue in: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585756.html Well, it prevents the infinite loop testcase from segfaulting when the function infinite() is not declared with const attribute explicitely but it is only internally auto-discovered to be ECF_CONST. I think that is OK because explicit attribute const should guarantee no side-effects and infinite loop is IMHO one (though I have read how we document it in our manual and I am not entirely sure that is how we handle it, but IMHO the manual disallows infinite looping too). (We also discussed the post-dominance check with Honza in person and at least my impression is that the situation is not as dire as comment #9 might suggest.)
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267
--- Comment #9 from hubicka at kam dot mff.cuni.cz ---
> @@ -1,4 +1,3 @@
> -static int
> __attribute__ ((noinline,const))
> infinite (int p)
> {
Just for a record, it crahes with or without static int here for me :)
I run across it because the code tracking must access in ipa-sra is IMO
conceptually wrong. I noticed that because ipa-modref solves similar
problem for kills (both need to verify that given access will always
happen). The post-dominance check is not enough to verify that because
earlier function calls can do things like EH. I failed to construct an
actual testcase because on interesting stuff like EH we punt for other
reasons (missed fnspec annotations on EH builtins). I will play with it
more today.
Re: [Bug ipa/103267] Wrong code with ipa-sra
> @@ -1,4 +1,3 @@
> -static int
> __attribute__ ((noinline,const))
> infinite (int p)
> {
Just for a record, it crahes with or without static int here for me :)
I run across it because the code tracking must access in ipa-sra is IMO
conceptually wrong. I noticed that because ipa-modref solves similar
problem for kills (both need to verify that given access will always
happen). The post-dominance check is not enough to verify that because
earlier function calls can do things like EH. I failed to construct an
actual testcase because on interesting stuff like EH we punt for other
reasons (missed fnspec annotations on EH builtins). I will play with it
more today.
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267
--- Comment #8 from Martin Liška ---
(In reply to hubicka from comment #6)
> Aha, but here is better example (reproduces same way).
> In the former one I forgot const attribute which makes it invalid.
> The testcase tests that ipa-sra is missing ECF_LOOPING_CONST_OR_PURE
> check
>
> static int
> __attribute__ ((noinline))
> infinite (int p)
> {
> if (p)
> while (1);
> return p;
> }
> __attribute__ ((noinline))
> static void
> test(int p, int *a)
> {
> int v = infinite (p);
> if (*a && v)
> __builtin_abort ();
> }
> test2(int *a)
> {
> test(0,a);
> }
> main()
> {
> test (1,0);
> }
This one is very old :) Also 4.8.0 crashes.
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 Martin Liška changed: What|Removed |Added Status|WAITING |NEW
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267
--- Comment #7 from Martin Liška ---
Now, I can reproduce it, the original code snippet was different:
diff -u pr103267-o.c pr103267.c
--- pr103267-o.c2021-11-16 09:47:41.463349286 +0100
+++ pr103267.c 2021-11-16 09:47:11.811550854 +0100
@@ -1,4 +1,3 @@
-static int
__attribute__ ((noinline,const))
infinite (int p)
{
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267
--- Comment #6 from hubicka at kam dot mff.cuni.cz ---
Aha, but here is better example (reproduces same way).
In the former one I forgot const attribute which makes it invalid.
The testcase tests that ipa-sra is missing ECF_LOOPING_CONST_OR_PURE
check
static int
__attribute__ ((noinline))
infinite (int p)
{
if (p)
while (1);
return p;
}
__attribute__ ((noinline))
static void
test(int p, int *a)
{
int v = infinite (p);
if (*a && v)
__builtin_abort ();
}
test2(int *a)
{
test(0,a);
}
main()
{
test (1,0);
}
Re: [Bug ipa/103267] Wrong code with ipa-sra
Aha, but here is better example (reproduces same way).
In the former one I forgot const attribute which makes it invalid.
The testcase tests that ipa-sra is missing ECF_LOOPING_CONST_OR_PURE
check
static int
__attribute__ ((noinline))
infinite (int p)
{
if (p)
while (1);
return p;
}
__attribute__ ((noinline))
static void
test(int p, int *a)
{
int v = infinite (p);
if (*a && v)
__builtin_abort ();
}
test2(int *a)
{
test(0,a);
}
main()
{
test (1,0);
}
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267
--- Comment #5 from hubicka at kam dot mff.cuni.cz ---
Works for me even with the 3 warnings.
hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ cat >tt.c
__attribute__ ((noinline,const))
infinite (int p)
{
if (p)
while (1);
return p;
}
__attribute__ ((noinline))
static void
test(int p, int *a)
{
int v = infinite (p);
if (*a && v)
__builtin_abort ();
}
test2(int *a)
{
test(0,a);
}
main()
{
test (1,0);
}
hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./xgcc --version
xgcc (GCC) 12.0.0 2024 (experimental)
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./xgcc -B ./ -O2 tt.c
tt.c:2:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
2 | infinite (int p)
| ^~~~
tt.c:16:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
16 | test2(int *a)
| ^
tt.c:20:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
20 | main()
| ^~~~
hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./a.out
Segmentation fault
Re: [Bug ipa/103267] Wrong code with ipa-sra
Works for me even with the 3 warnings.
hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ cat >tt.c
__attribute__ ((noinline,const))
infinite (int p)
{
if (p)
while (1);
return p;
}
__attribute__ ((noinline))
static void
test(int p, int *a)
{
int v = infinite (p);
if (*a && v)
__builtin_abort ();
}
test2(int *a)
{
test(0,a);
}
main()
{
test (1,0);
}
hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./xgcc --version
xgcc (GCC) 12.0.0 2024 (experimental)
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./xgcc -B ./ -O2 tt.c
tt.c:2:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
2 | infinite (int p)
| ^~~~
tt.c:16:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
16 | test2(int *a)
| ^
tt.c:20:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
20 | main()
| ^~~~
hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./a.out
Segmentation fault
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 --- Comment #4 from Martin Liška --- Still, I can't reproduce with the current master. Apparently, the code snippet from #c0 produces only 2 warnings: $ gcc pr103267.c -c -O2 && ./a.out pr103267.c:17:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 17 | test2(int *a) | ^ pr103267.c:21:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 21 | main() | ^~~~
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 --- Comment #3 from Martin Liška --- Ah, ok, so no ICE, but wrong code.
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 --- Comment #2 from Jan Hubicka --- jan@localhost:~> gcc t.c t.c:2:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 2 | infinite (int p) | ^~~~ t.c:16:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 16 | test2(int *a) | ^ t.c:20:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 20 | main() | ^~~~ jan@localhost:~> ./a.out ...loops infinitely jan@localhost:~> gcc t.c -O2 t.c:2:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 2 | infinite (int p) | ^~~~ t.c:16:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 16 | test2(int *a) | ^ t.c:20:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 20 | main() | ^~~~ jan@localhost:~> ./a.out Segmentation fault (core dumped)
[Bug ipa/103267] Wrong code with ipa-sra
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267 Martin Liška changed: What|Removed |Added Last reconfirmed||2021-11-16 Status|UNCONFIRMED |WAITING Ever confirmed|0 |1 --- Comment #1 from Martin Liška --- How do we ICE, I can't reproduce it.
