| Message ID | 20240129160026.562739-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Generate REG_CFA_UNDEFINED for unsaved callee-saved registers | expand |
On Mon, Jan 29, 2024 at 08:00:26AM -0800, H.J. Lu wrote: > Attach REG_CFA_UNDEFINED notes for unsaved callee-saved registers which > have been used in the function to an instruction in prologue. > > gcc/ > > PR target/38534 > * dwarf2cfi.cc (add_cfi_undefined): New. > (dwarf2out_frame_debug_cfa_undefined): Likewise. > (dwarf2out_frame_debug): Handle REG_CFA_UNDEFINED. > * reg-notes.def (REG_CFA_UNDEFINED): New. > * config/i386/i386.cc (ix86_expand_prologue): Attach > REG_CFA_UNDEFINED notes for unsaved callee-saved registers > which have been used in the function to an instruction in > prologue. > > 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 | 20 +++++++ > gcc/dwarf2cfi.cc | 55 +++++++++++++++++++ > gcc/reg-notes.def | 4 ++ > .../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 +++++ > 7 files changed, 139 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..6ec87b6a16f 100644 > --- a/gcc/config/i386/i386.cc > +++ b/gcc/config/i386/i386.cc > @@ -9304,6 +9304,26 @@ ix86_expand_prologue (void) > combined with prologue modifications. */ > if (TARGET_SEH) > emit_insn (gen_prologue_use (stack_pointer_rtx)); > + > + if (cfun->machine->call_saved_registers > + != TYPE_NO_CALLEE_SAVED_REGISTERS) > + return; > + > + insn = get_insns (); > + if (!insn) > + return; You can't attach the notes to a random instruction that happens to be first in the function. 1) it needs to be a real instruction, not a note 2) it needs to be RTX_FRAME_RELATED_P 3) if it is RTX_FRAME_RELATED_P, but doesn't contain any previous REG_CFA_* notes: 3a) if it has REG_FRAME_RELATED_EXPR note, then I believe just that note argument is processed instead of the instruction pattern and I think REG_CFA_* notes which precede REG_FRAME_RELATED_EXPR are processed, but REG_CFA_* notes after it are not; so adding REG_CFA_UNDEFINED notes at least if the adding is after the existing notes instead of before them may be problematic 3b) if it has neither REG_CFA_* nor REG_FRAME_RELATED_EXPR notes, then normally the pattern of the insn would be processed in dwarf2cfi. But with the REG_CFA_* notes that part will be ignored. > --- a/gcc/dwarf2cfi.cc > +++ b/gcc/dwarf2cfi.cc > @@ -517,6 +517,17 @@ add_cfi_restore (unsigned reg) > add_cfi (cfi); > } > Function comment missing. > +static void > +add_cfi_undefined (unsigned reg) > +{ > + dw_cfi_ref cfi = new_cfi (); > + > + cfi->dw_cfi_opc = DW_CFA_undefined; > + cfi->dw_cfi_oprnd1.dw_cfi_reg_num = reg; > + > + add_cfi (cfi); > +} > + > /* Perform ROW->REG_SAVE[COLUMN] = CFI. CFI may be null, indicating > that the register column is no longer saved. */ Jakub
On Mon, Jan 29, 2024 at 8:30 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Mon, Jan 29, 2024 at 08:00:26AM -0800, H.J. Lu wrote: > > Attach REG_CFA_UNDEFINED notes for unsaved callee-saved registers which > > have been used in the function to an instruction in prologue. > > > > gcc/ > > > > PR target/38534 > > * dwarf2cfi.cc (add_cfi_undefined): New. > > (dwarf2out_frame_debug_cfa_undefined): Likewise. > > (dwarf2out_frame_debug): Handle REG_CFA_UNDEFINED. > > * reg-notes.def (REG_CFA_UNDEFINED): New. > > * config/i386/i386.cc (ix86_expand_prologue): Attach > > REG_CFA_UNDEFINED notes for unsaved callee-saved registers > > which have been used in the function to an instruction in > > prologue. > > > > 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 | 20 +++++++ > > gcc/dwarf2cfi.cc | 55 +++++++++++++++++++ > > gcc/reg-notes.def | 4 ++ > > .../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 +++++ > > 7 files changed, 139 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..6ec87b6a16f 100644 > > --- a/gcc/config/i386/i386.cc > > +++ b/gcc/config/i386/i386.cc > > @@ -9304,6 +9304,26 @@ ix86_expand_prologue (void) > > combined with prologue modifications. */ > > if (TARGET_SEH) > > emit_insn (gen_prologue_use (stack_pointer_rtx)); > > + > > + if (cfun->machine->call_saved_registers > > + != TYPE_NO_CALLEE_SAVED_REGISTERS) > > + return; > > + > > + insn = get_insns (); > > + if (!insn) > > + return; > > You can't attach the notes to a random instruction that happens to be first > in the function. > 1) it needs to be a real instruction, not a note Will fix it. > 2) it needs to be RTX_FRAME_RELATED_P This should work: insn = nullptr; rtx_insn *next; for (next = get_insns (); next; next = NEXT_INSN (next)) { if (!RTX_FRAME_RELATED_P (next)) continue; insn = next; } if (!insn) return; > 3) if it is RTX_FRAME_RELATED_P, but doesn't contain any previous REG_CFA_* > notes: > 3a) if it has REG_FRAME_RELATED_EXPR note, then I believe just that > note argument is processed instead of the instruction pattern and > I think REG_CFA_* notes which precede REG_FRAME_RELATED_EXPR are > processed, but REG_CFA_* notes after it are not; so adding > REG_CFA_UNDEFINED notes at least if the adding is after the existing > notes instead of before them may be problematic Since register note is added to the head: /* Add register note with kind KIND and datum DATUM to INSN. */ void add_reg_note (rtx insn, enum reg_note kind, rtx datum) { REG_NOTES (insn) = alloc_reg_note (kind, datum, REG_NOTES (insn)); } it isn't an issue. > 3b) if it has neither REG_CFA_* nor REG_FRAME_RELATED_EXPR notes, then > normally the pattern of the insn would be processed in dwarf2cfi. > But with the REG_CFA_* notes that part will be ignored. > > > --- a/gcc/dwarf2cfi.cc > > +++ b/gcc/dwarf2cfi.cc > > @@ -517,6 +517,17 @@ add_cfi_restore (unsigned reg) > > add_cfi (cfi); > > } > > > > Function comment missing. Will fix it in the v3 patch. Thanks. > > +static void > > +add_cfi_undefined (unsigned reg) > > +{ > > + dw_cfi_ref cfi = new_cfi (); > > + > > + cfi->dw_cfi_opc = DW_CFA_undefined; > > + cfi->dw_cfi_oprnd1.dw_cfi_reg_num = reg; > > + > > + add_cfi (cfi); > > +} > > + > > /* Perform ROW->REG_SAVE[COLUMN] = CFI. CFI may be null, indicating > > that the register column is no longer saved. */ > > Jakub >
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index b3e7c74846e..6ec87b6a16f 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -9304,6 +9304,26 @@ ix86_expand_prologue (void) combined with prologue modifications. */ if (TARGET_SEH) emit_insn (gen_prologue_use (stack_pointer_rtx)); + + if (cfun->machine->call_saved_registers + != TYPE_NO_CALLEE_SAVED_REGISTERS) + return; + + insn = get_insns (); + if (!insn) + return; + + /* Attach REG_CFA_UNDEFINED notes for unsaved callee-saved registers + which have been used in the function to an instruction in prologue. + */ + 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)) + add_reg_note (insn, REG_CFA_UNDEFINED, + gen_rtx_REG (word_mode, i)); } /* Emit code to restore REG using a POP or POPP insn. */ diff --git a/gcc/dwarf2cfi.cc b/gcc/dwarf2cfi.cc index 1231b5bb5f0..12862ed1070 100644 --- a/gcc/dwarf2cfi.cc +++ b/gcc/dwarf2cfi.cc @@ -517,6 +517,17 @@ add_cfi_restore (unsigned reg) add_cfi (cfi); } +static void +add_cfi_undefined (unsigned reg) +{ + dw_cfi_ref cfi = new_cfi (); + + cfi->dw_cfi_opc = DW_CFA_undefined; + cfi->dw_cfi_oprnd1.dw_cfi_reg_num = reg; + + add_cfi (cfi); +} + /* Perform ROW->REG_SAVE[COLUMN] = CFI. CFI may be null, indicating that the register column is no longer saved. */ @@ -1532,6 +1543,37 @@ dwarf2out_frame_debug_cfa_restore (rtx reg, bool emit_cfi) } } +/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_UNDEFINED + note. */ + +static void +dwarf2out_frame_debug_cfa_undefined (rtx reg) +{ + gcc_assert (REG_P (reg)); + + rtx span = targetm.dwarf_register_span (reg); + if (!span) + { + unsigned int regno = dwf_regno (reg); + add_cfi_undefined (regno); + } + else + { + /* We have a PARALLEL describing where the contents of REG live. + Restore the register for each piece of the PARALLEL. */ + gcc_assert (GET_CODE (span) == PARALLEL); + + const int par_len = XVECLEN (span, 0); + for (int par_index = 0; par_index < par_len; par_index++) + { + reg = XVECEXP (span, 0, par_index); + gcc_assert (REG_P (reg)); + unsigned int regno = dwf_regno (reg); + add_cfi_undefined (regno); + } + } +} + /* A subroutine of dwarf2out_frame_debug, process a REG_CFA_WINDOW_SAVE. ??? Perhaps we should note in the CIE where windows are saved (instead @@ -2326,6 +2368,19 @@ dwarf2out_frame_debug (rtx_insn *insn) handled_one = true; break; + case REG_CFA_UNDEFINED: + n = XEXP (note, 0); + if (n == nullptr) + { + n = PATTERN (insn); + if (GET_CODE (n) == PARALLEL) + n = XVECEXP (n, 0, 0); + n = XEXP (n, 0); + } + dwarf2out_frame_debug_cfa_undefined (n); + handled_one = true; + break; + case REG_CFA_SET_VDRAP: n = XEXP (note, 0); if (REG_P (n)) diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def index 5b878fb2a1c..8a78ebb6864 100644 --- a/gcc/reg-notes.def +++ b/gcc/reg-notes.def @@ -152,6 +152,10 @@ REG_CFA_NOTE (CFA_EXPRESSION) the given register. */ REG_CFA_NOTE (CFA_VAL_EXPRESSION) +/* Attached to insns that are RTX_FRAME_RELATED_P, to specific a register + with undefined value. */ +REG_CFA_NOTE (CFA_UNDEFINED) + /* Attached to insns that are RTX_FRAME_RELATED_P, with the information that this is a restore operation, i.e. will result in DW_CFA_restore or the like. Either the attached rtx, or the destination of the insn's 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..578a093e0ca --- /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 3" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 6" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 12" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 13" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 14" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 15" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 3" 1 { target ia32 } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 5" 1 { target ia32 } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 6" 1 { target ia32 } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 7" 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..4a0d399d904 --- /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 3" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 6" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 12" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 13" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 14" 1 { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 15" 1 { target lp64 } } } */ +/* { dg-final { scan-assembler-not ".cfi_undefined 15" { target x32 } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 3" 1 { target ia32 } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 5" 1 { target ia32 } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 6" 1 { target ia32 } } } */ +/* { dg-final { scan-assembler-times ".cfi_undefined 7" 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" } } */