diff mbox series

Don't emit "parameter passing ABI changes in -fabi-version=12 (GCC 8)" warnings for internal linkage functions (PR c++/89105)

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

Commit Message

Jakub Jelinek Jan. 29, 2019, 6:31 p.m. UTC
Hi!

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.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

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.


	Jakub

Comments

Jason Merrill Jan. 29, 2019, 9:29 p.m. UTC | #1
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
Jakub Jelinek Jan. 29, 2019, 9:49 p.m. UTC | #2
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
Jason Merrill Jan. 29, 2019, 10:30 p.m. UTC | #3
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
diff mbox series

Patch

--- 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" }
+}