From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47566 invoked by alias); 5 Feb 2020 22:52:02 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 47540 invoked by uid 89); 5 Feb 2020 22:52:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-ot1-f65.google.com Received: from mail-ot1-f65.google.com (HELO mail-ot1-f65.google.com) (209.85.210.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Feb 2020 22:52:00 +0000 Received: by mail-ot1-f65.google.com with SMTP id a15so3662871otf.1 for ; Wed, 05 Feb 2020 14:52:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=8J2IFpWa/ZSBmtHj6peM0UXDHVEuQOsHY/v77BYX1WI=; b=GXbbB4YwdSgamhJ7yKgQ8MRiFhuPKu/lAOYSuY9INsCKp/cIGzCRo3uQHT177DDjtk ENzcNM/6kkBfUNHSQiSjaIqxMEpU4F/hXO88Zg1tizRKnXJuJ2p0KSAGBl8IPmWPhdR+ wCeoGwuuzlLumRnmwKqsMarB5Mntu2NDSKrBNCNzA4D9jLAMcfEyCdAN+WjDjAGjvQ9D 2zOuIaP864fnRjm1eegikRN0l5GnODdb0keLUovpG51EVwz/S2mycjtosAC6wmm4E6tF 38xCslmDV0K0pm+5+sMoxm84qLGnW4is3xgJ/Y0I0jVsoH/mgTSl6k3gUg+9Jnz2ZBIF M/Gw== MIME-Version: 1.0 References: <20200205143300.144541-1-hjl.tools@gmail.com> <20200205143300.144541-3-hjl.tools@gmail.com> <20200205223737.GF1941471@redhat.com> In-Reply-To: <20200205223737.GF1941471@redhat.com> From: "H.J. Lu" Date: Wed, 05 Feb 2020 22:52:00 -0000 Message-ID: Subject: Re: [PATCH] Add patch_area_size and patch_area_entry to crtl To: Marek Polacek Cc: GCC Patches , Uros Bizjak , Jeff Law , Richard Biener , Richard Earnshaw , Jakub Jelinek , Torsten Duwe , Szabolcs Nagy , Eric Botcazou , Richard Sandiford Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg00305.txt.bz2 On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek wrote: > > On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote: > > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu wrote: > > > > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford > > > wrote: > > > > > > > > "H.J. Lu" writes: > > > > > Currently patchable area is at the wrong place. > > > > > > > > Agreed :-) > > > > > > > > > It is placed immediately > > > > > after function label and before .cfi_startproc. A backend should= be able > > > > > to add a pseudo patchable area instruction durectly into RTL. Th= is patch > > > > > adds patch_area_size and patch_area_entry to cfun so that the pat= chable > > > > > area info is available in RTL passes. > > > > > > > > It might be better to add it to crtl, since it should only be needed > > > > during rtl generation. > > > > > > > > > It also limits patch_area_size and patch_area_entry to 65535, whi= ch is > > > > > a reasonable maximum size for patchable area. > > > > > > > > > > gcc/ > > > > > > > > > > PR target/93492 > > > > > * function.c (expand_function_start): Set cfun->patch_area_= size > > > > > and cfun->patch_area_entry. > > > > > * function.h (function): Add patch_area_size and patch_area= _entry. > > > > > * opts.c (common_handle_option): Limit > > > > > function_entry_patch_area_size and function_entry_patch_are= a_start > > > > > to USHRT_MAX. Fix a typo in error message. > > > > > * varasm.c (assemble_start_function): Use cfun->patch_area_= size > > > > > and cfun->patch_area_entry. > > > > > * doc/invoke.texi: Document the maximum value for > > > > > -fpatchable-function-entry. > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > PR target/93492 > > > > > * c-c++-common/patchable_function_entry-error-1.c: New test. > > > > > * c-c++-common/patchable_function_entry-error-2.c: Likewise. > > > > > * c-c++-common/patchable_function_entry-error-3.c: Likewise. > > > > > --- > > > > > gcc/doc/invoke.texi | 1 + > > > > > gcc/function.c | 35 +++++++++++++= ++++++ > > > > > gcc/function.h | 6 ++++ > > > > > gcc/opts.c | 4 ++- > > > > > .../patchable_function_entry-error-1.c | 9 +++++ > > > > > .../patchable_function_entry-error-2.c | 9 +++++ > > > > > .../patchable_function_entry-error-3.c | 20 +++++++++++ > > > > > gcc/varasm.c | 30 ++-----------= --- > > > > > 8 files changed, 85 insertions(+), 29 deletions(-) > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function= _entry-error-1.c > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function= _entry-error-2.c > > > > > create mode 100644 gcc/testsuite/c-c++-common/patchable_function= _entry-error-3.c > > > > > > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > > > > > index 35b341e759f..dd4835199b0 100644 > > > > > --- a/gcc/doc/invoke.texi > > > > > +++ b/gcc/doc/invoke.texi > > > > > @@ -13966,6 +13966,7 @@ If @code{N=3D0}, no pad location is recor= ded. > > > > > The NOP instructions are inserted at---and maybe before, dependi= ng on > > > > > @var{M}---the function entry address, even before the prologue. > > > > > > > > > > +The maximum value of @var{N} and @var{M} is 65535. > > > > > @end table > > > > > > > > > > > > > > > diff --git a/gcc/function.c b/gcc/function.c > > > > > index d8008f60422..badbf538eec 100644 > > > > > --- a/gcc/function.c > > > > > +++ b/gcc/function.c > > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr) > > > > > /* If we are doing generic stack checking, the probe should go= here. */ > > > > > if (flag_stack_check =3D=3D GENERIC_STACK_CHECK) > > > > > stack_check_probe_note =3D emit_note (NOTE_INSN_DELETED); > > > > > + > > > > > + unsigned HOST_WIDE_INT patch_area_size =3D function_entry_patc= h_area_size; > > > > > + unsigned HOST_WIDE_INT patch_area_entry =3D function_entry_pat= ch_area_start; > > > > > + > > > > > + tree patchable_function_entry_attr > > > > > + =3D lookup_attribute ("patchable_function_entry", > > > > > + DECL_ATTRIBUTES (cfun->decl)); > > > > > + if (patchable_function_entry_attr) > > > > > + { > > > > > + tree pp_val =3D TREE_VALUE (patchable_function_entry_attr); > > > > > + tree patchable_function_entry_value1 =3D TREE_VALUE (pp_va= l); > > > > > + > > > > > + patch_area_size =3D tree_to_uhwi (patchable_function_entry= _value1); > > > > > + patch_area_entry =3D 0; > > > > > + if (TREE_CHAIN (pp_val) !=3D NULL_TREE) > > > > > + { > > > > > + tree patchable_function_entry_value2 > > > > > + =3D TREE_VALUE (TREE_CHAIN (pp_val)); > > > > > + patch_area_entry =3D tree_to_uhwi (patchable_function_ent= ry_value2); > > > > > + } > > > > > + if (patch_area_size > USHRT_MAX || patch_area_size > USHRT= _MAX) > > > > > + error ("invalid values for % att= ribute"); > > > > > > > > This should probably go in handle_patchable_function_entry_attribute > > > > instead. It doesn't look like we have a clear policy about whether > > > > errors or warnings are right for unrecognised arguments to known > > > > attribute names, but handle_patchable_function_entry_attribute repo= rts > > > > an OPT_Wattributes warning for arguments that aren't integers. Doi= ng the > > > > same for out-of-range integers would be more consistent and means t= hat > > > > we wouldn't break existing code if we relaxed/changed the rules in = future. > > > > > > Like this? OK for master if there is no regression? > > > > > > > There is a small problem. Warnings from C and C++ frond-ends are diffe= rent: > > > > [hjl@gnu-skx-1 gcc]$ cat x.c > > void > > __attribute__((patchable_function_entry(65536))) > > foo1 (void) > > { /* { dg-warning "'patchable_function_entry' attribute argument > > '65536' is out of range" } */ > > } > > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c > > x.c:4:1: warning: =E2=80=98patchable_function_entry=E2=80=99 attribute = argument > > =E2=80=9865536=E2=80=99 is out of range (> 65535) [-Wattributes] > > 4 | { /* { dg-warning "'patchable_function_entry' attribute > > argument '65536' is out of range" } */ > > | ^ > > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c > > x.c:3:11: warning: =E2=80=98patchable_function_entry=E2=80=99 attribute= argument > > =E2=80=9865536=E2=80=99 is out of range (> 65535) [-Wattributes] > > 3 | foo1 (void) > > | ^ > > [hjl@gnu-skx-1 gcc]$ > > > > C warns at line 4 and C++ warns at line 3. Do I need separate tests > > for C and C++? > > I think better would be > > /* { dg-error "foo" "" { target c } } */ > /* { dg-error "bar" "" { target c++ } .-1 } */ > > Marek > It worked. Thanks. --=20 H.J.