Message ID | 20190129183136.GS2135@tucnak |
---|---|
State | New |
Headers | show |
Series | Don't emit "parameter passing ABI changes in -fabi-version=12 (GCC 8)" warnings for internal linkage functions (PR c++/89105) | expand |
On Tue, Jan 29, 2019 at 1:38 PM Jakub Jelinek <jakub@redhat.com> wrote: > > Emitting this warning for internal linkage functions makes no sense to me, > the ABI of those functions is solely under control of the compiler that > knows the callee as well as all callers and can do anything it wants. > I've added DECL_PRESERVE_P to the test, so if somebody uses used attribute > and accesses those from inline assembly, they get warning. Calling an affected function from inline assembly seems extremely unlikely, so I'd probably just check TREE_PUBLIC. Why do you also check DECL_EXTERNAL? Jason
On Tue, Jan 29, 2019 at 04:29:59PM -0500, Jason Merrill wrote: > On Tue, Jan 29, 2019 at 1:38 PM Jakub Jelinek <jakub@redhat.com> wrote: > > > > Emitting this warning for internal linkage functions makes no sense to me, > > the ABI of those functions is solely under control of the compiler that > > knows the callee as well as all callers and can do anything it wants. > > I've added DECL_PRESERVE_P to the test, so if somebody uses used attribute > > and accesses those from inline assembly, they get warning. > > Calling an affected function from inline assembly seems extremely Ok, can drop that. > unlikely, so I'd probably just check TREE_PUBLIC. Why do you also > check DECL_EXTERNAL? I thought TREE_PUBLIC might not be set on DECL_EXTERNAL fndecls, but it seems it is set. So, like this then (if it passes bootstrap/regtest)? 2019-01-29 Jakub Jelinek <jakub@redhat.com> PR c++/89105 * config/i386/i386.c (ix86_warn_parameter_passing_abi): Don't warn for arguments to functions that are TU-local and shouldn't be referenced by assembly. * g++.target/i386/pr89105.C: New test. --- gcc/config/i386/i386.c.jj 2019-01-24 21:20:06.902275003 +0100 +++ gcc/config/i386/i386.c 2019-01-29 16:50:20.157550206 +0100 @@ -29562,6 +29562,10 @@ ix86_warn_parameter_passing_abi (cumulat if (!TYPE_EMPTY_P (type)) return; + /* Don't warn if the function isn't visible outside of the TU. */ + if (cum->decl && !TREE_PUBLIC (cum->decl)) + return; + const_tree ctx = get_ultimate_context (cum->decl); if (ctx != NULL_TREE && !TRANSLATION_UNIT_WARN_EMPTY_P (ctx)) --- gcc/testsuite/g++.target/i386/pr89105.C.jj 2019-01-29 16:53:23.692535030 +0100 +++ gcc/testsuite/g++.target/i386/pr89105.C 2019-01-29 16:53:59.952939332 +0100 @@ -0,0 +1,16 @@ +// PR c++/89105 +// { dg-do compile { target c++11 } } +// { dg-options "-fabi-version=12 -Wabi=11" } + +namespace { + template<typename F> + void run(F f, int i) // { dg-bogus "parameter passing ABI changes in -fabi-version=12" } + { + f(i); + } +} + +void f() +{ + run([](int) { }, 1); // { dg-bogus "parameter passing ABI changes in -fabi-version=12" } +} Jakub
On Tue, Jan 29, 2019 at 4:49 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Jan 29, 2019 at 04:29:59PM -0500, Jason Merrill wrote: > > On Tue, Jan 29, 2019 at 1:38 PM Jakub Jelinek <jakub@redhat.com> wrote: > > > > > > Emitting this warning for internal linkage functions makes no sense to me, > > > the ABI of those functions is solely under control of the compiler that > > > knows the callee as well as all callers and can do anything it wants. > > > I've added DECL_PRESERVE_P to the test, so if somebody uses used attribute > > > and accesses those from inline assembly, they get warning. > > > > Calling an affected function from inline assembly seems extremely > > Ok, can drop that. > > > unlikely, so I'd probably just check TREE_PUBLIC. Why do you also > > check DECL_EXTERNAL? > > I thought TREE_PUBLIC might not be set on DECL_EXTERNAL fndecls, but it > seems it is set. > > So, like this then (if it passes bootstrap/regtest)? OK. Jason
--- gcc/config/i386/i386.c.jj 2019-01-24 21:20:06.902275003 +0100 +++ gcc/config/i386/i386.c 2019-01-29 16:50:20.157550206 +0100 @@ -29562,6 +29562,14 @@ ix86_warn_parameter_passing_abi (cumulat if (!TYPE_EMPTY_P (type)) return; + /* Don't warn if the function isn't visible outside of the + TU and can't be accessed by assembly either. */ + if (cum->decl + && !(TREE_PUBLIC (cum->decl) + || DECL_EXTERNAL (cum->decl) + || DECL_PRESERVE_P (cum->decl))) + return; + const_tree ctx = get_ultimate_context (cum->decl); if (ctx != NULL_TREE && !TRANSLATION_UNIT_WARN_EMPTY_P (ctx)) --- gcc/testsuite/g++.target/i386/pr89105.C.jj 2019-01-29 16:53:23.692535030 +0100 +++ gcc/testsuite/g++.target/i386/pr89105.C 2019-01-29 16:53:59.952939332 +0100 @@ -0,0 +1,16 @@ +// PR c++/89105 +// { dg-do compile { target c++11 } } +// { dg-options "-fabi-version=12 -Wabi=11" } + +namespace { + template<typename F> + void run(F f, int i) // { dg-bogus "parameter passing ABI changes in -fabi-version=12" } + { + f(i); + } +} + +void f() +{ + run([](int) { }, 1); // { dg-bogus "parameter passing ABI changes in -fabi-version=12" } +}