On Fri, Mar 19, 2021 at 1:39 AM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> On Thu, Mar 18, 2021 at 11:44 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > If we never generate function body, we shouldn't issue errors for return
> > nor argument.  Add init_cumulative_args_called to i386 machine_function
> > to avoid issuing errors for return and argument without function body.
> >
> > gcc/
> >
> >         PR target/99652
> >         * config/i386/i386.c (init_cumulative_args): Set
> >         init_cumulative_args_called to true.
> >         (construct_container): Issue error for return and argument only
> >         if init_cumulative_args_called is true.
> >         * config/i386/i386.h (machine_function): Add
> >         init_cumulative_args_called.
> >
> > gcc/testsuite/
> >
> >         PR target/99652
> >         * gcc.dg/torture/pr99652-1.c: New test.
> >         * gcc.dg/torture/pr99652-2.c: Likewise.
> >         * gcc.target/i386/pr57655.c: Adjusted.
> >         * gcc.target/i386/pr59794-6.c: Likewise.
> >         * gcc.target/i386/pr70738-1.c: Likewise.
> >         * gcc.target/i386/pr96744-1.c: Likewise.
> > ---
> >  gcc/config/i386/i386.c                    | 26 ++++++++++++++---------
> >  gcc/config/i386/i386.h                    |  3 +++
> >  gcc/testsuite/gcc.dg/torture/pr99652-1.c  |  8 +++++++
> >  gcc/testsuite/gcc.dg/torture/pr99652-2.c  |  8 +++++++
> >  gcc/testsuite/gcc.target/i386/pr57655.c   |  4 ++--
> >  gcc/testsuite/gcc.target/i386/pr59794-6.c |  4 ++--
> >  gcc/testsuite/gcc.target/i386/pr70738-1.c |  4 ++--
> >  gcc/testsuite/gcc.target/i386/pr96744-1.c |  4 ++--
> >  8 files changed, 43 insertions(+), 18 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/torture/pr99652-1.c
> >  create mode 100644 gcc/testsuite/gcc.dg/torture/pr99652-2.c
> >
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index 540d4f44517..4a0b8c73bef 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -1705,6 +1705,8 @@ init_cumulative_args (CUMULATIVE_ARGS *cum,  /* 
> > Argument info to initialize */
> >    struct cgraph_node *local_info_node = NULL;
> >    struct cgraph_node *target = NULL;
> >
> > +  cfun->machine->init_cumulative_args_called = true;
> > +
> >    memset (cum, 0, sizeof (*cum));
> >
> >    if (fndecl)
> > @@ -2534,18 +2536,21 @@ construct_container (machine_mode mode, 
> > machine_mode orig_mode,
> >       some less clueful developer tries to use floating-point anyway.  */
> >    if (needed_sseregs && !TARGET_SSE)
> >      {
> > -      if (in_return)
> > +      if (cfun->machine->init_cumulative_args_called)
>
> Please make this an early return, with appropriate comment.

Done.

> >         {
> > -         if (!issued_sse_ret_error)
> > +         if (in_return)
> >             {
> > -             error ("SSE register return with SSE disabled");
> > -             issued_sse_ret_error = true;
> > +             if (!issued_sse_ret_error)
> > +               {
> > +                 error ("SSE register return with SSE disabled");
> > +                 issued_sse_ret_error = true;
> > +               }
> > +           }
> > +         else if (!issued_sse_arg_error)
> > +           {
> > +             error ("SSE register argument with SSE disabled");
> > +             issued_sse_arg_error = true;
> >             }
> > -       }
> > -      else if (!issued_sse_arg_error)
> > -       {
> > -         error ("SSE register argument with SSE disabled");
> > -         issued_sse_arg_error = true;
> >         }
> >        return NULL;
> >      }
> > @@ -2558,7 +2563,8 @@ construct_container (machine_mode mode, machine_mode 
> > orig_mode,
> >           || regclass[i] == X86_64_X87UP_CLASS
> >           || regclass[i] == X86_64_COMPLEX_X87_CLASS)
> >         {
> > -         if (!issued_x87_ret_error)
> > +         if (cfun->machine->init_cumulative_args_called
> > +             && !issued_x87_ret_error)
>
> Also, early return.

Done.

> >             {
> >               error ("x87 register return with x87 disabled");
> >               issued_x87_ret_error = true;
> > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> > index 48749104b24..ad908c010b3 100644
> > --- a/gcc/config/i386/i386.h
> > +++ b/gcc/config/i386/i386.h
> > @@ -2945,6 +2945,9 @@ struct GTY(()) machine_function {
> >       function.  */
> >    BOOL_BITFIELD has_explicit_vzeroupper : 1;
> >
> > +  /* If true if init_cumulative_args has been called.  */
> > +  BOOL_BITFIELD init_cumulative_args_called: 1;
>
> I think that we should follow aarch64 lead and name it silent_p. The
> comment also suits our needs.
>
>   bool silent_p;        /* True if we should act silently, rather than
>                    raise an error for invalid calls.  */
>

Done.

Here is the v2 patch.  OK for master?

Thanks.

-- 
H.J.
From 1cca62e74aa9e443acc491d8fa830ecb2aa4eede Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Thu, 18 Mar 2021 11:47:46 -0700
Subject: [PATCH v2] x86: Issue error for return/argument only with function
 body

If we never generate function body, we shouldn't issue errors for return
nor argument.  Add silent_p to i386 machine_function to avoid issuing
errors for return and argument without function body.

gcc/

	PR target/99652
	* config/i386/i386-options.c (ix86_init_machine_status): Set
	silent_p to true.
	* config/i386/i386.c (init_cumulative_args): Set silent_p to
	false.
	(construct_container): Return early for return and argument
	errors if silent_p is true.
	* config/i386/i386.h (machine_function): Add silent_p.

gcc/testsuite/

	PR target/99652
	* gcc.dg/torture/pr99652-1.c: New test.
	* gcc.dg/torture/pr99652-2.c: Likewise.
	* gcc.target/i386/pr57655.c: Adjusted.
	* gcc.target/i386/pr59794-6.c: Likewise.
	* gcc.target/i386/pr70738-1.c: Likewise.
	* gcc.target/i386/pr96744-1.c: Likewise.

Fix
---
 gcc/config/i386/i386-options.c            |  1 +
 gcc/config/i386/i386.c                    | 12 ++++++++++++
 gcc/config/i386/i386.h                    |  4 ++++
 gcc/testsuite/gcc.dg/torture/pr99652-1.c  |  8 ++++++++
 gcc/testsuite/gcc.dg/torture/pr99652-2.c  |  8 ++++++++
 gcc/testsuite/gcc.target/i386/pr57655.c   |  4 ++--
 gcc/testsuite/gcc.target/i386/pr59794-6.c |  4 ++--
 gcc/testsuite/gcc.target/i386/pr70738-1.c |  4 ++--
 gcc/testsuite/gcc.target/i386/pr96744-1.c |  4 ++--
 9 files changed, 41 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr99652-1.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr99652-2.c

diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
index 7865bc110a3..b653527d266 100644
--- a/gcc/config/i386/i386-options.c
+++ b/gcc/config/i386/i386-options.c
@@ -1768,6 +1768,7 @@ ix86_init_machine_status (void)
   f = ggc_cleared_alloc<machine_function> ();
   f->call_abi = ix86_abi;
   f->stack_frame_required = true;
+  f->silent_p = true;
 
   return f;
 }
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 540d4f44517..714349094bd 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1705,6 +1705,10 @@ init_cumulative_args (CUMULATIVE_ARGS *cum,  /* Argument info to initialize */
   struct cgraph_node *local_info_node = NULL;
   struct cgraph_node *target = NULL;
 
+  /* Set silent_p to false to raise an error for invalid calls when
+     expanding function body.  */
+  cfun->machine->silent_p = false;
+
   memset (cum, 0, sizeof (*cum));
 
   if (fndecl)
@@ -2534,6 +2538,10 @@ construct_container (machine_mode mode, machine_mode orig_mode,
      some less clueful developer tries to use floating-point anyway.  */
   if (needed_sseregs && !TARGET_SSE)
     {
+      /* Return early if we shouldn't raise an error for invalid
+	 calls.  */
+      if (cfun->machine->silent_p)
+	return NULL;
       if (in_return)
 	{
 	  if (!issued_sse_ret_error)
@@ -2558,6 +2566,10 @@ construct_container (machine_mode mode, machine_mode orig_mode,
 	  || regclass[i] == X86_64_X87UP_CLASS
 	  || regclass[i] == X86_64_COMPLEX_X87_CLASS)
 	{
+	  /* Return early if we shouldn't raise an error for invalid
+	     calls.  */
+	  if (cfun->machine->silent_p)
+	    return NULL;
 	  if (!issued_x87_ret_error)
 	    {
 	      error ("x87 register return with x87 disabled");
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 48749104b24..9974c10cd3b 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2945,6 +2945,10 @@ struct GTY(()) machine_function {
      function.  */
   BOOL_BITFIELD has_explicit_vzeroupper : 1;
 
+  /* If true if we should act silently, rather than raise an error for
+     invalid calls.  */
+  BOOL_BITFIELD silent_p : 1;
+
   /* The largest alignment, in bytes, of stack slot actually used.  */
   unsigned int max_used_stack_alignment;
 
diff --git a/gcc/testsuite/gcc.dg/torture/pr99652-1.c b/gcc/testsuite/gcc.dg/torture/pr99652-1.c
new file mode 100644
index 00000000000..c2395ff4ed8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr99652-1.c
@@ -0,0 +1,8 @@
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-mgeneral-regs-only" } */
+
+inline double
+foo (void)
+{
+  return 1.0;
+}
diff --git a/gcc/testsuite/gcc.dg/torture/pr99652-2.c b/gcc/testsuite/gcc.dg/torture/pr99652-2.c
new file mode 100644
index 00000000000..beefad8bfee
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr99652-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-mno-80387" } */
+
+inline double
+foo (void)
+{
+  return 1.0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr57655.c b/gcc/testsuite/gcc.target/i386/pr57655.c
index 33a59d3a263..649cdef832d 100644
--- a/gcc/testsuite/gcc.target/i386/pr57655.c
+++ b/gcc/testsuite/gcc.target/i386/pr57655.c
@@ -2,7 +2,7 @@
 /* { dg-options "-mavx -mvzeroupper -mno-fp-ret-in-387" } */
 
 long double
-foo (long double x)
-{ /* { dg-error "x87 register return with x87 disabled" "" { target { ! ia32 } } } */
+foo (long double x) /* { dg-error "x87 register return with x87 disabled" "" { target { ! ia32 } } } */
+{
   return __builtin_ilogbl (x);
 }
diff --git a/gcc/testsuite/gcc.target/i386/pr59794-6.c b/gcc/testsuite/gcc.target/i386/pr59794-6.c
index c809f957927..babcf76eaf8 100644
--- a/gcc/testsuite/gcc.target/i386/pr59794-6.c
+++ b/gcc/testsuite/gcc.target/i386/pr59794-6.c
@@ -8,7 +8,7 @@ typedef int __v4si __attribute__ ((__vector_size__ (16)));
 extern __v4si x;
 
 __v4si
-foo (void)
-{ /* { dg-error "SSE register return with SSE disabled" } */
+foo (void) /* { dg-error "SSE register return with SSE disabled" } */
+{
   return x;
 }
diff --git a/gcc/testsuite/gcc.target/i386/pr70738-1.c b/gcc/testsuite/gcc.target/i386/pr70738-1.c
index 19381c26932..62d609c9f66 100644
--- a/gcc/testsuite/gcc.target/i386/pr70738-1.c
+++ b/gcc/testsuite/gcc.target/i386/pr70738-1.c
@@ -3,7 +3,7 @@
 
 typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
 
-int32x2_t test (int32x2_t a, int32x2_t b)
-{ /* { dg-error "SSE register return with SSE disabled" } */
+int32x2_t test (int32x2_t a, int32x2_t b) /* { dg-error "SSE register return with SSE disabled" } */
+{
   return a + b;
 }
diff --git a/gcc/testsuite/gcc.target/i386/pr96744-1.c b/gcc/testsuite/gcc.target/i386/pr96744-1.c
index 46f3ce6ddd4..da5557d89b7 100644
--- a/gcc/testsuite/gcc.target/i386/pr96744-1.c
+++ b/gcc/testsuite/gcc.target/i386/pr96744-1.c
@@ -4,7 +4,7 @@
 typedef int int32x2_t __attribute__ ((__vector_size__ ((8))));
 
 __attribute__((__target__("general-regs-only")))
-int32x2_t test (int32x2_t a, int32x2_t b)
-{ /* { dg-error "SSE register return with SSE disabled" } */
+int32x2_t test (int32x2_t a, int32x2_t b) /* { dg-error "SSE register return with SSE disabled" } */
+{
   return a + b;
 }
-- 
2.30.2

Reply via email to