| Message ID | 20240127204124.393779-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Generate .cfi_undefined for unsaved callee-saved registers | expand |
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
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.
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" } } */