[PATCH] x86: Generate .cfi_undefined for unsaved callee-saved registers

[PATCH] x86: Generate .cfi_undefined for unsaved callee-saved registers

[H.J. Lu]

When assembler directives for DWARF frame unwind is enabled, generate
the .cfi_undefined directive for unsaved callee-saved registers which
have been used in the function.

gcc/

	PR target/38534
	* config/i386/i386.cc (ix86_post_cfi_startproc): New.
	(TARGET_ASM_POST_CFI_STARTPROC): Likewise.

gcc/testsuite/

	PR target/38534
	* gcc.target/i386/no-callee-saved-19.c: New test.
	* gcc.target/i386/no-callee-saved-20.c: Likewise.
	* gcc.target/i386/pr38534-7.c: Likewise.
	* gcc.target/i386/pr38534-8.c: Likewise.
---
 gcc/config/i386/i386.cc                       | 37 +++++++++++++++++++
 .../gcc.target/i386/no-callee-saved-19.c      | 17 +++++++++
 .../gcc.target/i386/no-callee-saved-20.c      | 12 ++++++
 gcc/testsuite/gcc.target/i386/pr38534-7.c     | 18 +++++++++
 gcc/testsuite/gcc.target/i386/pr38534-8.c     | 13 +++++++
 5 files changed, 97 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-19.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-callee-saved-20.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr38534-7.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr38534-8.c

diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index b3e7c74846e..d4c10a5ef9b 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -22662,6 +22662,40 @@ x86_output_mi_thunk (FILE *file, tree thunk_fndecl, HOST_WIDE_INT delta,
   flag_force_indirect_call = saved_flag_force_indirect_call;
 }
 
+/* Implement TARGET_ASM_POST_CFI_STARTPROC.  Triggered after a
+   .cfi_startproc directive is emitted into the assembly file.
+   When assembler directives for DWARF frame unwind is enabled,
+   output the .cfi_undefined directive for unsaved callee-saved
+   registers which have been used in the function.  */
+
+void
+ix86_post_cfi_startproc (FILE *f, tree)
+{
+  if ((cfun->machine->call_saved_registers
+       == TYPE_NO_CALLEE_SAVED_REGISTERS)
+      && dwarf2out_do_cfi_asm ())
+    for (int i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+      if (df_regs_ever_live_p (i)
+	  && !fixed_regs[i]
+	  && !call_used_regs[i]
+	  && !STACK_REGNO_P (i)
+	  && !MMX_REGNO_P (i))
+	{
+	  if (LEGACY_INT_REGNO_P (i))
+	    {
+	      if (TARGET_64BIT)
+		asm_fprintf (f, "\t.cfi_undefined r%s\n",
+			     hi_reg_name[i]);
+	      else
+		asm_fprintf (f, "\t.cfi_undefined e%s\n",
+			     hi_reg_name[i]);
+	    }
+	  else
+	    asm_fprintf (f, "\t.cfi_undefined %s\n",
+			 hi_reg_name[i]);
+	}
+}
+
 static void
 x86_file_start (void)
 {
@@ -26281,6 +26315,9 @@ static const scoped_attribute_specs *const ix86_attribute_table[] =
 #undef TARGET_ASM_CAN_OUTPUT_MI_THUNK
 #define TARGET_ASM_CAN_OUTPUT_MI_THUNK x86_can_output_mi_thunk
 
+#undef TARGET_ASM_POST_CFI_STARTPROC
+#define TARGET_ASM_POST_CFI_STARTPROC ix86_post_cfi_startproc
+
 #undef TARGET_ASM_FILE_START
 #define TARGET_ASM_FILE_START x86_file_start
 
diff --git a/gcc/testsuite/gcc.target/i386/no-callee-saved-19.c b/gcc/testsuite/gcc.target/i386/no-callee-saved-19.c
new file mode 100644
index 00000000000..60a492cffd3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/no-callee-saved-19.c
@@ -0,0 +1,17 @@
+/* { dg-do assemble { target *-*-linux* *-*-gnu* } } */
+/* { dg-options "-save-temps -march=tigerlake -O2 -mtune-ctrl=^prologue_using_move,^epilogue_using_move" } */
+
+#include "no-callee-saved-1.c"
+
+/* { dg-final { scan-assembler-not "push" } } */
+/* { dg-final { scan-assembler-not "pop" } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined rbx" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined rbp" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined r12" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined r13" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined r14" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined r15" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined ebx" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined esi" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined edi" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined ebp" 1 { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/no-callee-saved-20.c b/gcc/testsuite/gcc.target/i386/no-callee-saved-20.c
new file mode 100644
index 00000000000..fc94778824a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/no-callee-saved-20.c
@@ -0,0 +1,12 @@
+/* { dg-do compile { target cfi } } */
+/* { dg-options "-march=tigerlake -O2 -mtune-ctrl=^prologue_using_move,^epilogue_using_move" } */
+
+__attribute__ ((no_callee_saved_registers))
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler-not "push" } } */
+/* { dg-final { scan-assembler-not "pop" } } */
+/* { dg-final { scan-assembler-not ".cfi_undefined" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr38534-7.c b/gcc/testsuite/gcc.target/i386/pr38534-7.c
new file mode 100644
index 00000000000..01dbd77cd75
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr38534-7.c
@@ -0,0 +1,18 @@
+/* { dg-do assemble { target *-*-linux* *-*-gnu* } } */
+/* { dg-options "-save-temps -march=tigerlake -O2 -mtune-ctrl=^prologue_using_move,^epilogue_using_move" } */
+
+#include "pr38534-1.c"
+
+/* { dg-final { scan-assembler-not "push" } } */
+/* { dg-final { scan-assembler-not "pop" } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined rbx" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined rbp" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined r12" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined r13" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined r14" 1 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined r15" 1 { target lp64 } } } */
+/* { dg-final { scan-assembler-not ".cfi_undefined r15" { target x32 } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined ebx" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined esi" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined edi" 1 { target ia32 } } } */
+/* { dg-final { scan-assembler-times ".cfi_undefined ebp" 1 { target ia32 } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr38534-8.c b/gcc/testsuite/gcc.target/i386/pr38534-8.c
new file mode 100644
index 00000000000..020c1512db1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr38534-8.c
@@ -0,0 +1,13 @@
+/* { dg-do compile { target cfi } } */
+/* { dg-options "-march=tigerlake -O2 -mtune-ctrl=^prologue_using_move,^epilogue_using_move" } */
+
+void
+__attribute__((noreturn))
+no_return_to_caller (void)
+{
+  while (1);
+}
+
+/* { dg-final { scan-assembler-not "push" } } */
+/* { dg-final { scan-assembler-not "pop" } } */
+/* { dg-final { scan-assembler-not ".cfi_undefined" } } */
-- 
2.43.0

Re: [PATCH] x86: Generate .cfi_undefined for unsaved callee-saved registers

[Jakub Jelinek]

On Sat, Jan 27, 2024 at 12:41:24PM -0800, H.J. Lu wrote:
> When assembler directives for DWARF frame unwind is enabled, generate
> the .cfi_undefined directive for unsaved callee-saved registers which
> have been used in the function.
> 
> gcc/
> 
> 	PR target/38534
> 	* config/i386/i386.cc (ix86_post_cfi_startproc): New.
> 	(TARGET_ASM_POST_CFI_STARTPROC): Likewise.
> 
> gcc/testsuite/
> 
> 	PR target/38534
> 	* gcc.target/i386/no-callee-saved-19.c: New test.
> 	* gcc.target/i386/no-callee-saved-20.c: Likewise.
> 	* gcc.target/i386/pr38534-7.c: Likewise.
> 	* gcc.target/i386/pr38534-8.c: Likewise.

This only works for -fdwarf2-cfi-asm, but doesn't work for
-fno-dwarf2-cfi-asm.  I think we need something that will work for both.

So, I'd say we want to add support for REG_CFA_UNDEFINED note, emit those
notes on some frame related insn in the prologue during prologue expansion
in pro_and_epilogue pass and handle that in dwarf2cfi.cc pass.

One question is where those should be emitted.  Emitting them right
at the start of the function has an advantage that it can be emitted in
CIE for all FDEs of noreturn functions (or with the new attribute).  But
disadvantage is of course that it will make e.g. debugging experience worse
even in the prologues of functions where the callee saved registers which
current function actually doesn't save aren't modified yet.
E.g. for the cases where callee saved registers are saved to memory or
registers I think dwarf2cfi.cc attempts to optimize and move the .cfi_*
directives or .eh_frame record later into the function as long as the
corresponding original register isn't modified yet.  Perhaps that should
be done also for the undefined case, ideally by using the same dwarf2cfi.cc
code.  So just perhaps at the start of the function read in the
REG_CFA_UNDEFINED notes for all the ever modified callee saved registers
which won't be actually saved and turn that into similar record like for
the saving into stack or other regs, just noting it is undefined instead
and have it pushed later as much as possible.

	Jakub

Re: [PATCH] x86: Generate .cfi_undefined for unsaved callee-saved registers

[H.J. Lu]

On Mon, Jan 29, 2024 at 2:08 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Sat, Jan 27, 2024 at 12:41:24PM -0800, H.J. Lu wrote:
> > When assembler directives for DWARF frame unwind is enabled, generate
> > the .cfi_undefined directive for unsaved callee-saved registers which
> > have been used in the function.
> >
> > gcc/
> >
> >       PR target/38534
> >       * config/i386/i386.cc (ix86_post_cfi_startproc): New.
> >       (TARGET_ASM_POST_CFI_STARTPROC): Likewise.
> >
> > gcc/testsuite/
> >
> >       PR target/38534
> >       * gcc.target/i386/no-callee-saved-19.c: New test.
> >       * gcc.target/i386/no-callee-saved-20.c: Likewise.
> >       * gcc.target/i386/pr38534-7.c: Likewise.
> >       * gcc.target/i386/pr38534-8.c: Likewise.
>
> This only works for -fdwarf2-cfi-asm, but doesn't work for
> -fno-dwarf2-cfi-asm.  I think we need something that will work for both.

-fno-dwarf2-cfi-asm stops generating all CFI directives.

> So, I'd say we want to add support for REG_CFA_UNDEFINED note, emit those
> notes on some frame related insn in the prologue during prologue expansion
> in pro_and_epilogue pass and handle that in dwarf2cfi.cc pass.

It is a good idea.  Here is the patch:

https://patchwork.sourceware.org/project/gcc/list/?series=30314

> One question is where those should be emitted.  Emitting them right
> at the start of the function has an advantage that it can be emitted in
> CIE for all FDEs of noreturn functions (or with the new attribute).  But
> disadvantage is of course that it will make e.g. debugging experience worse
> even in the prologues of functions where the callee saved registers which
> current function actually doesn't save aren't modified yet.
> E.g. for the cases where callee saved registers are saved to memory or
> registers I think dwarf2cfi.cc attempts to optimize and move the .cfi_*
> directives or .eh_frame record later into the function as long as the
> corresponding original register isn't modified yet.  Perhaps that should
> be done also for the undefined case, ideally by using the same dwarf2cfi.cc
> code.  So just perhaps at the start of the function read in the
> REG_CFA_UNDEFINED notes for all the ever modified callee saved registers
> which won't be actually saved and turn that into similar record like for
> the saving into stack or other regs, just noting it is undefined instead
> and have it pushed later as much as possible.

My patch doesn't implement this optimization.

-- 
H.J.