Re: [PATCH] [RFC] PR target/52813 and target/11807

[Dimitar Dimitrov <dimitar@dinux.eu>]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]

On Sun, Dec 16 2018 at 14:36:26 EET Bernd Edlinger wrote:
> Hi,
> 
> if I understood that right, then clobbering sp is and has always been
> ignored.
>
> If that is right, then I would much prefer a warning, that says exactly
> that, because that would also help to understand why removing that clobber
> statement is safe even for old gcc versions.
> 
> Since your patch did not actually change the handling of the PIC register,
> that one should of course stay an error.

Thank you. Looks like general consensus is to have a warning. See attached 
patch that switches the error to a warning.

Regards,
Dimitar

[-- Attachment #2: 0001-PR-target-52813.patch --]
[-- Type: text/x-patch, Size: 2298 bytes --]

From d589ebd7824b4505ab75a2404f49a7c200679545 Mon Sep 17 00:00:00 2001
From: Dimitar Dimitrov <dimitar@dinux.eu>
Date: Sun, 16 Dec 2018 10:13:18 +0200
Subject: [PATCH] PR target/52813

Turns out there are existing programs that clobber stack pointer.
To avoid disruption, switch the newly introduced error to a warning.

Tested with:
  $ make check-gcc-c RUNTESTFLAGS="i386.exp=pr52813.c "

gcc/ChangeLog:

2018-12-16  Dimitar Dimitrov  <dimitar@dinux.eu>

	* cfgexpand.c (asm_clobber_reg_is_valid): Switch error to warning.
	Add clarification why there is a warning.

gcc/testsuite/ChangeLog:

2018-12-16  Dimitar Dimitrov  <dimitar@dinux.eu>

	* gcc.target/i386/pr52813.c (test1): Update warning message.

Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
---
 gcc/cfgexpand.c                         | 7 +++++--
 gcc/testsuite/gcc.target/i386/pr52813.c | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 0d04bbcafce..1e44c9a7ad0 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2872,10 +2872,13 @@ asm_clobber_reg_is_valid (int regno, int nregs, const char *regname)
       error ("PIC register clobbered by %qs in %<asm%>", regname);
       is_valid = false;
     }
-  /* Clobbering the STACK POINTER register is an error.  */
+  /* Clobbered STACK POINTER register is not saved/restored by GCC,
+     which is often unexpected by users.  See PR52813.  */
   if (overlaps_hard_reg_set_p (regset, Pmode, STACK_POINTER_REGNUM))
     {
-      error ("Stack Pointer register clobbered by %qs in %<asm%>", regname);
+      warning (0, "Stack Pointer register clobbered by %qs in %<asm%>",
+	       regname);
+      warning (0, "GCC has always ignored Stack Pointer %<asm%> clobbers");
       is_valid = false;
     }
 
diff --git a/gcc/testsuite/gcc.target/i386/pr52813.c b/gcc/testsuite/gcc.target/i386/pr52813.c
index 154ebbfc423..644fef15fef 100644
--- a/gcc/testsuite/gcc.target/i386/pr52813.c
+++ b/gcc/testsuite/gcc.target/i386/pr52813.c
@@ -5,5 +5,5 @@
 void
 test1 (void)
 {
-  asm volatile ("" : : : "%esp"); /* { dg-error "Stack Pointer register clobbered" } */
+  asm volatile ("" : : : "%esp"); /* { dg-warning "Stack Pointer register clobbered.\+GCC has always ignored Stack Pointer 'asm' clobbers" } */
 }
-- 
2.11.0