On Thu, Jun 11, 2020 at 8:24 PM Jeff Law <l...@redhat.com> wrote:
>
> On Sat, 2020-05-02 at 04:55 -0700, H.J. Lu wrote:
> > Currently patchable area is at the wrong place. It is placed immediately
> > after function label, before both .cfi_startproc and ENDBR. This patch
> > adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
> > changes ENDBR insertion pass to also insert patchable area instruction.
> > TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing
> > patchable area before .cfi_startproc and ENDBR.
> >
> > OK for master?
> >
> > Thanks.
> >
> > H.J.
> > ---
> > gcc/
> >
> > PR target/93492
> > * config/i386/i386-features.c (rest_of_insert_endbranch):
> > Renamed to ...
> > (rest_of_insert_endbr_and_patchable_area): Change return type
> > to void. Add need_endbr and patchable_area_size arguments.
> > Don't call timevar_push nor timevar_pop. Replace
> > endbr_queued_at_entrance with insn_queued_at_entrance. Insert
> > UNSPECV_PATCHABLE_AREA for patchable area.
> > (pass_data_insert_endbranch): Renamed to ...
> > (pass_data_insert_endbr_and_patchable_area): This. Change
> > pass name to endbr_and_patchable_area.
> > (pass_insert_endbranch): Renamed to ...
> > (pass_insert_endbr_and_patchable_area): This. Add need_endbr
> > and patchable_area_size;.
> > (pass_insert_endbr_and_patchable_area::gate): Set and check
> > need_endbr and patchable_area_size.
> > (pass_insert_endbr_and_patchable_area::execute): Call
> > timevar_push and timevar_pop. Pass need_endbr and
> > patchable_area_size to rest_of_insert_endbr_and_patchable_area.
> > (make_pass_insert_endbranch): Renamed to ...
> > (make_pass_insert_endbr_and_patchable_area): This.
> > * config/i386/i386-passes.def: Replace pass_insert_endbranch
> > with pass_insert_endbr_and_patchable_area.
> > * config/i386/i386-protos.h (ix86_output_patchable_area): New.
> > (make_pass_insert_endbranch): Renamed to ...
> > (make_pass_insert_endbr_and_patchable_area): This.
> > * config/i386/i386.c (ix86_asm_output_function_label): Set
> > function_label_emitted to true.
> > (ix86_print_patchable_function_entry): New function.
> > (ix86_output_patchable_area): Likewise.
> > (x86_function_profiler): Replace endbr_queued_at_entrance with
> > insn_queued_at_entrance. Generate ENDBR only for TYPE_ENDBR.
> > Call ix86_output_patchable_area to generate patchable area if
> > needed.
> > (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
> > * i386.h (queued_insn_type): New.
> > (machine_function): Add function_label_emitted. Replace
> > endbr_queued_at_entrance with insn_queued_at_entrance.
> > * config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
> > (patchable_area): New.
> >
> > gcc/testsuite/
> >
> > PR target/93492
> > * gcc.target/i386/pr93492-1.c: New test.
> > * gcc.target/i386/pr93492-2.c: Likewise.
> > * gcc.target/i386/pr93492-3.c: Likewise.
> > * gcc.target/i386/pr93492-4.c: Likewise.
> > * gcc.target/i386/pr93492-5.c: Likewise.
> OK
> jeff
> >
My patch triggered a latent x86 backend bug. This patch fixes
it. OK for master?
Thanks.
--
H.J.
From 0a5ebc4daad8533271406a4713059c19ebc76f56 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Fri, 12 Jun 2020 05:44:59 -0700
Subject: [PATCH] Linux/i386: Remove SUBTARGET_FRAME_POINTER_REQUIRED
config/i386/gnu-user.h has
#define SUBTARGET_FRAME_POINTER_REQUIRED crtl->profile
ix86_frame_pointer_required() has
/* Several x86 os'es need a frame pointer for other reasons,
usually pertaining to setjmp. */
if (SUBTARGET_FRAME_POINTER_REQUIRED)
return true;
...
if (crtl->profile && !flag_fentry)
return true;
A frame pointer is needed only for -pg, not for -mfentry -pg. Remove
SUBTARGET_FRAME_POINTER_REQUIRED from gnu-user.h to make i386 GCC behave
the same as x86-64 GCC. This fixes
FAIL: gcc.target/i386/pr93492-3.c scan-assembler \t.cfi_startproc\n\tendbr(32|64)\n.*.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n
FAIL: gcc.target/i386/pr93492-5.c scan-assembler \t.cfi_startproc\n.*.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n
on Linux/i386.
PR target/95655
* config/i386/gnu-user.h (SUBTARGET_FRAME_POINTER_REQUIRED):
Removed.
* config/i386/i386.c (ix86_frame_pointer_required): Update
comments.
---
gcc/config/i386/gnu-user.h | 6 ------
gcc/config/i386/i386.c | 2 ++
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/gcc/config/i386/gnu-user.h b/gcc/config/i386/gnu-user.h
index ae4aa844f02..6ec5a114270 100644
--- a/gcc/config/i386/gnu-user.h
+++ b/gcc/config/i386/gnu-user.h
@@ -39,12 +39,6 @@ along with GCC; see the file COPYING3. If not see
#undef MCOUNT_NAME
#define MCOUNT_NAME "mcount"
-/* The GLIBC version of mcount for the x86 assumes that there is a
- frame, so we cannot allow profiling without a frame pointer. */
-
-#undef SUBTARGET_FRAME_POINTER_REQUIRED
-#define SUBTARGET_FRAME_POINTER_REQUIRED crtl->profile
-
#undef SIZE_TYPE
#define SIZE_TYPE "unsigned int"
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3b776c08a22..b84c6133ed0 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -5267,6 +5267,8 @@ ix86_frame_pointer_required (void)
|| ix86_current_function_calls_tls_descriptor))
return true;
+ /* Several versions of mcount for the x86 assumes that there is a
+ frame, so we cannot allow profiling without a frame pointer. */
if (crtl->profile && !flag_fentry)
return true;
--
2.26.2
