public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: "Jose E. Marchesi" <jose.marchesi@oracle.com>
Cc: David Faust <david.faust@oracle.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] bpf: fix pseudoc w regs for small modes [PR111029]
Date: Thu, 17 Aug 2023 10:44:23 +0200	[thread overview]
Message-ID: <CAFiYyc0kvcsVv2GX2Cu+HEKM1wGrh4dRP6_RNqEJw01nh6hx_Q@mail.gmail.com> (raw)
In-Reply-To: <87cyzo3z8c.fsf@oracle.com>

On Tue, Aug 15, 2023 at 9:03 PM Jose E. Marchesi via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> Hello David.
> Thanks for the patch.
>
> OK.

Picking a random patch/mail for this question - how do we maintain BPF
support for the most recent GCC release which is GCC 13?  I see the
current state in GCC 13 isn't fully able to provide upstream kernel BPF
support but GCC 14 contains some bugfixes and some new features(?).
Is it worthwhile to backport at least bugfixes while GCC 14 is still in
development even if those are not regression fixes?  Or is GCC 13 BPF
too broken to be used anyway?

Thanks,
Richard.

> > In the BPF pseudo-c assembly dialect, registers treated as 32-bits
> > rather than the full 64 in various instructions ought to be printed as
> > "wN" rather than "rN".  But bpf_print_register () was only doing this
> > for specifically SImode registers, meaning smaller modes were printed
> > incorrectly.
> >
> > This caused assembler errors like:
> >
> >   Error: unrecognized instruction `w2 =(s8)r1'
> >
> > for a 32-bit sign-extending register move instruction, where the source
> > register is used in QImode.
> >
> > Fix bpf_print_register () to print the "w" version of register when
> > specified by the template for any mode 32-bits or smaller.
> >
> > Tested on bpf-unknown-none.
> >
> >       PR target/111029
> >
> > gcc/
> >       * config/bpf/bpf.cc (bpf_print_register): Print 'w' registers
> >       for any mode 32-bits or smaller, not just SImode.
> >
> > gcc/testsuite/
> >
> >       * gcc.target/bpf/smov-2.c: New test.
> >       * gcc.target/bpf/smov-pseudoc-2.c: New test.
> > ---
> >  gcc/config/bpf/bpf.cc                         |  2 +-
> >  gcc/testsuite/gcc.target/bpf/smov-2.c         | 15 +++++++++++++++
> >  gcc/testsuite/gcc.target/bpf/smov-pseudoc-2.c | 15 +++++++++++++++
> >  3 files changed, 31 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.target/bpf/smov-2.c
> >  create mode 100644 gcc/testsuite/gcc.target/bpf/smov-pseudoc-2.c
> >
> > diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> > index 3516b79bce4..1d0abd7fbb3 100644
> > --- a/gcc/config/bpf/bpf.cc
> > +++ b/gcc/config/bpf/bpf.cc
> > @@ -753,7 +753,7 @@ bpf_print_register (FILE *file, rtx op, int code)
> >      fprintf (file, "%s", reg_names[REGNO (op)]);
> >    else
> >      {
> > -      if (code == 'w' && GET_MODE (op) == SImode)
> > +      if (code == 'w' && GET_MODE_SIZE (GET_MODE (op)) <= 4)
> >       {
> >         if (REGNO (op) == BPF_FP)
> >           fprintf (file, "w10");
> > diff --git a/gcc/testsuite/gcc.target/bpf/smov-2.c b/gcc/testsuite/gcc.target/bpf/smov-2.c
> > new file mode 100644
> > index 00000000000..6f3516d2385
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/bpf/smov-2.c
> > @@ -0,0 +1,15 @@
> > +/* Check signed 32-bit mov instructions.  */
> > +/* { dg-do compile } */
> > +/* { dg-options "-mcpu=v4 -O2" } */
> > +
> > +int
> > +foo (unsigned char a, unsigned short b)
> > +{
> > +  int x = (char) a;
> > +  int y = (short) b;
> > +
> > +  return x + y;
> > +}
> > +
> > +/* { dg-final { scan-assembler {movs32\t%r.,%r.,8\n} } } */
> > +/* { dg-final { scan-assembler {movs32\t%r.,%r.,16\n} } } */
> > diff --git a/gcc/testsuite/gcc.target/bpf/smov-pseudoc-2.c b/gcc/testsuite/gcc.target/bpf/smov-pseudoc-2.c
> > new file mode 100644
> > index 00000000000..6af6cadf8df
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/bpf/smov-pseudoc-2.c
> > @@ -0,0 +1,15 @@
> > +/* Check signed 32-bit mov instructions (pseudo-C asm dialect).  */
> > +/* { dg-do compile } */
> > +/* { dg-options "-mcpu=v4 -O2 -masm=pseudoc" } */
> > +
> > +int
> > +foo (unsigned char a, unsigned short b)
> > +{
> > +  int x = (char) a;
> > +  int y = (short) b;
> > +
> > +  return x + y;
> > +}
> > +
> > +/* { dg-final { scan-assembler {w. = \(s8\) w.\n} } } */
> > +/* { dg-final { scan-assembler {w. = \(s16\) w.\n} } } */

  reply	other threads:[~2023-08-17  8:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-15 18:46 David Faust
2023-08-15 19:02 ` Jose E. Marchesi
2023-08-17  8:44   ` Richard Biener [this message]
2023-08-17  9:15     ` Jose E. Marchesi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFiYyc0kvcsVv2GX2Cu+HEKM1wGrh4dRP6_RNqEJw01nh6hx_Q@mail.gmail.com \
    --to=richard.guenther@gmail.com \
    --cc=david.faust@oracle.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jose.marchesi@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).