diff mbox

Avoid weird -Wpsabi warnings from NRV (PR target/80310)

Message ID 20170404192919.GM17461@tucnak
State New
Headers show

Commit Message

Jakub Jelinek April 4, 2017, 7:29 p.m. UTC 
Hi!

When I've implemented vector -fsanitize=signed-integer-overflow,
we've started to emit weirdo warnings on e.g. the following testcase.

There is really no call with return value that changes ABI
based on ISA options, there is just an internal fn call that is expanded
completely inline (UBSAN_CHECK_*), but as the NRV pass calls
aggregate_value_p even on those internal functions, we get the warning.

The following patch keeps doing that for internal functions that have
corresponding optab, those typically have a library counterpart, but
for others that really don't have one and are always expanded inline
avoids calling aggregate_value_p, we really can't NRV optimize those
internal fns anyway.

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

2017-04-04  Jakub Jelinek  <jakub@redhat.com>

	PR target/80310
	* tree-nvr.c: Include internal-fn.h.
	(pass_return_slot::execute): Ignore internal calls without
	direct optab.

	* c-c++-common/ubsan/pr80310.c: New test.


	Jakub

Comments

Richard Biener April 4, 2017, 8:38 p.m. UTC | #1 
On April 4, 2017 9:29:19 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>When I've implemented vector -fsanitize=signed-integer-overflow,
>we've started to emit weirdo warnings on e.g. the following testcase.
>
>There is really no call with return value that changes ABI
>based on ISA options, there is just an internal fn call that is
>expanded
>completely inline (UBSAN_CHECK_*), but as the NRV pass calls
>aggregate_value_p even on those internal functions, we get the warning.
>
>The following patch keeps doing that for internal functions that have
>corresponding optab, those typically have a library counterpart, but
>for others that really don't have one and are always expanded inline
>avoids calling aggregate_value_p, we really can't NRV optimize those
>internal fns anyway.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 

OK.

Richard.

>2017-04-04  Jakub Jelinek  <jakub@redhat.com>
>
>	PR target/80310
>	* tree-nvr.c: Include internal-fn.h.
>	(pass_return_slot::execute): Ignore internal calls without
>	direct optab.
>
>	* c-c++-common/ubsan/pr80310.c: New test.
>
>--- gcc/tree-nrv.c.jj	2017-01-01 12:45:39.000000000 +0100
>+++ gcc/tree-nrv.c	2017-04-04 15:51:54.864211786 +0200
>@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.
> #include "tree-pretty-print.h"
> #include "gimple-iterator.h"
> #include "gimple-walk.h"
>+#include "internal-fn.h"
> 
> /* This file implements return value optimizations for functions which
>    return aggregate types.
>@@ -377,6 +378,12 @@ pass_return_slot::execute (function *fun
> 	  if (stmt
> 	      && gimple_call_lhs (stmt)
> 	      && !gimple_call_return_slot_opt_p (stmt)
>+	      /* Ignore internal functions without direct optabs,
>+		 those are expanded specially and aggregate_value_p
>+		 on their result might result in undesirable warnings
>+		 with some backends.  */
>+	      && (!gimple_call_internal_p (stmt)
>+		  || direct_internal_fn_p (gimple_call_internal_fn (stmt)))
> 	      && aggregate_value_p (TREE_TYPE (gimple_call_lhs (stmt)),
> 				    gimple_call_fndecl (stmt)))
> 	    {
>--- gcc/testsuite/c-c++-common/ubsan/pr80310.c.jj	2017-04-04
>16:13:35.108407423 +0200
>+++ gcc/testsuite/c-c++-common/ubsan/pr80310.c	2017-04-04
>16:12:24.000000000 +0200
>@@ -0,0 +1,12 @@
>+/* PR target/80310 */
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -fsanitize=signed-integer-overflow" } */
>+/* { dg-additional-options "-mno-avx" { target i?86-*-* x86_64-*-* } }
>*/
>+
>+typedef int V __attribute__((vector_size (32)));
>+
>+void
>+foo (V *a, V *b, V *c)
>+{
>+  *a = *b + *c;		/* { dg-bogus "AVX vector return without AVX enabled
>changes the ABI" "" { target i?86-*-* x86_64-*-* } } */
>+}
>
>	Jakub
diff mbox

Patch

--- gcc/tree-nrv.c.jj	2017-01-01 12:45:39.000000000 +0100
+++ gcc/tree-nrv.c	2017-04-04 15:51:54.864211786 +0200
@@ -28,6 +28,7 @@  along with GCC; see the file COPYING3.
 #include "tree-pretty-print.h"
 #include "gimple-iterator.h"
 #include "gimple-walk.h"
+#include "internal-fn.h"
 
 /* This file implements return value optimizations for functions which
    return aggregate types.
@@ -377,6 +378,12 @@  pass_return_slot::execute (function *fun
 	  if (stmt
 	      && gimple_call_lhs (stmt)
 	      && !gimple_call_return_slot_opt_p (stmt)
+	      /* Ignore internal functions without direct optabs,
+		 those are expanded specially and aggregate_value_p
+		 on their result might result in undesirable warnings
+		 with some backends.  */
+	      && (!gimple_call_internal_p (stmt)
+		  || direct_internal_fn_p (gimple_call_internal_fn (stmt)))
 	      && aggregate_value_p (TREE_TYPE (gimple_call_lhs (stmt)),
 				    gimple_call_fndecl (stmt)))
 	    {
--- gcc/testsuite/c-c++-common/ubsan/pr80310.c.jj	2017-04-04 16:13:35.108407423 +0200
+++ gcc/testsuite/c-c++-common/ubsan/pr80310.c	2017-04-04 16:12:24.000000000 +0200
@@ -0,0 +1,12 @@ 
+/* PR target/80310 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fsanitize=signed-integer-overflow" } */
+/* { dg-additional-options "-mno-avx" { target i?86-*-* x86_64-*-* } } */
+
+typedef int V __attribute__((vector_size (32)));
+
+void
+foo (V *a, V *b, V *c)
+{
+  *a = *b + *c;		/* { dg-bogus "AVX vector return without AVX enabled changes the ABI" "" { target i?86-*-* x86_64-*-* } } */
+}