From: Conor Dooley <conor@kernel.org>
To: "Heiko Stübner" <heiko@sntech.de>
Cc: linux-riscv@lists.infradead.org, palmer@dabbelt.com,
christoph.muellner@vrull.eu, prabhakar.csengg@gmail.com,
philipp.tomsich@vrull.eu, ajones@ventanamicro.com,
emil.renner.berthing@canonical.com
Subject: Re: [PATCH 7/7] RISC-V: add zbb support to string functions
Date: Thu, 24 Nov 2022 22:32:58 +0000 [thread overview]
Message-ID: <Y3/xGhkbf6qEr+5r@spud> (raw)
In-Reply-To: <14728581.RDIVbhacDa@diego>
On Thu, Nov 24, 2022 at 11:23:08PM +0100, Heiko Stübner wrote:
> > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > index b22525290073..ac5555fd9788 100644
> > > --- a/arch/riscv/include/asm/hwcap.h
> > > +++ b/arch/riscv/include/asm/hwcap.h
> > > @@ -59,6 +59,7 @@ enum riscv_isa_ext_id {
> > > RISCV_ISA_EXT_ZIHINTPAUSE,
> > > RISCV_ISA_EXT_SSTC,
> > > RISCV_ISA_EXT_SVINVAL,
> > > + RISCV_ISA_EXT_ZBB,
> >
> > With ZIHINTPAUSE before SSTC and SVINIVAL I assume this is not something
> > we are canonically ordering but I never, ever know which ones we are
> > allowed to re-order at will.
>
> I guess we could extend the comments with suitable hints,
> which could help in the future.
Aye, for the likes of me that will never, ever remember I like the idea!
> > > RISCV_ISA_EXT_ID_MAX = RISCV_ISA_EXT_MAX,
> > > };
> >
> > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > > index bf9dd6764bad..66ff36a57e20 100644
> > > --- a/arch/riscv/kernel/cpu.c
> > > +++ b/arch/riscv/kernel/cpu.c
> > > @@ -166,6 +166,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = {
> > > __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> > > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> > > __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> > > + __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
> > > __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> > > __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
> > > __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
> >
> > This one I do know that Palmer wants canonically ordered.
btw, idk if you noticed but I appear to have picked canonical ordering
as today's thing to get confused about a lot.
You put zbb after the S extentions - does that meant it is *not* an
"Additional Standard Extension" but rather a regular Z one?
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index 026512ca9c4c..f19b9d4e2dca 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -201,6 +201,7 @@ void __init riscv_fill_hwcap(void)
> > > } else {
> > > SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
> > > SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> > > + SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
> > > SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
> > > SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
> > > SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
> >
> > This one looks like it is, sstc aside. Same question as above, can I
> > reorder this one? I'll send a patch for it if I can...
>
> hmm, I don't see the difference between cpu.c above
> (..., svpbmt, zbb, zicbom, ...) and here
> (..., svpbmt, zbb, zicbom, ...)
sstc appears last here but first in the cpu.c hunk above.
> > > diff --git a/arch/riscv/lib/strcmp_zbb.S b/arch/riscv/lib/strcmp_zbb.S
> > > new file mode 100644
> > > index 000000000000..aff9b941d3ee
> > > --- /dev/null
> > > +++ b/arch/riscv/lib/strcmp_zbb.S
> > > @@ -0,0 +1,91 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (c) 2022 VRULL GmbH
> > > + * Author: Christoph Muellner <christoph.muellner@vrull.eu>
> >
> > Is a Co-developed-by: appropriate then?
>
> I'd think so ... i.e. the assembly is from Christoph, but is originally
> part of a pending glibc patchset, hence Christoph and me
> decided on the co-developed thingy :-) .
Check your patch again, I don't see a Co-developed-by: tag. (That's what
I was getting at, not the validity of "Author: Christoph...")
> > > + /* Main loop for aligned string. */
> > > + .p2align 3
> > > +1:
> > > + REG_L data1, 0(src1)
> > > + REG_L data2, 0(src2)
> > > + orc.b data1_orcb, data1
> > > + bne data1_orcb, m1, 2f
> > > + addi src1, src1, SZREG
> > > + addi src2, src2, SZREG
> > > + beq data1, data2, 1b
> > > +
> > > + /* Words don't match, and no null byte in the first
> > > + * word. Get bytes in big-endian order and compare. */
> > > +#ifndef CONFIG_CPU_BIG_ENDIAN
> >
> > I know this is a lift from the reference implementation in the spec, but
> > do we actually need this ifndef section?
>
> I don't know, but _if_ big endian support comes in the future,
> having one place less to break also might be helpful? :-)
One less place to have to go and fix it up, but I hope it never comes to
pass! And no harm is being close to the one in the spec...
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2022-11-24 22:33 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-10 16:49 [PATCH 0/7] Zbb string optimizations and call support in alternatives Heiko Stuebner
2022-11-10 16:49 ` [PATCH 1/7] efi/riscv: libstub: mark when compiling libstub Heiko Stuebner
2022-11-13 17:16 ` Conor Dooley
2022-11-13 17:20 ` Heiko Stübner
2022-11-13 18:06 ` Conor Dooley
2022-11-10 16:49 ` [PATCH 2/7] RISC-V: add auipc elements to parse_asm header Heiko Stuebner
2022-11-13 17:18 ` Conor Dooley
2022-11-10 16:49 ` [PATCH 3/7] RISC-V: add U-type imm parsing " Heiko Stuebner
2022-11-13 19:06 ` Conor Dooley
2022-11-10 16:49 ` [PATCH 4/7] RISC-V: add rd reg " Heiko Stuebner
2022-11-13 19:08 ` Conor Dooley
2022-11-10 16:49 ` [PATCH 5/7] RISC-V: fix auipc-jalr addresses in patched alternatives Heiko Stuebner
2022-11-13 20:31 ` Conor Dooley
2022-11-14 10:57 ` Emil Renner Berthing
2022-11-14 11:35 ` Andrew Jones
2022-11-14 11:38 ` Emil Renner Berthing
2022-11-14 11:38 ` Heiko Stübner
2022-11-14 12:15 ` Andrew Jones
2022-11-14 12:29 ` Emil Renner Berthing
2022-11-14 12:47 ` Philipp Tomsich
2022-11-15 14:28 ` Lad, Prabhakar
2022-11-17 11:51 ` Heiko Stübner
2022-11-21 9:50 ` Lad, Prabhakar
2022-11-21 11:27 ` Heiko Stübner
2022-11-21 15:06 ` Lad, Prabhakar
2022-11-21 21:31 ` Lad, Prabhakar
2022-11-21 22:17 ` Heiko Stübner
2022-11-21 22:38 ` Heiko Stübner
2022-11-22 0:16 ` Lad, Prabhakar
2022-11-21 23:59 ` Lad, Prabhakar
2022-11-22 10:59 ` Lad, Prabhakar
2022-11-22 11:19 ` Heiko Stübner
2022-11-22 11:37 ` Heiko Stübner
2022-11-22 12:28 ` Lad, Prabhakar
2022-11-10 16:49 ` [PATCH 6/7] RISC-V: add infrastructure to allow different str* implementations Heiko Stuebner
2022-11-13 22:07 ` Conor Dooley
2022-11-10 16:49 ` [PATCH 7/7] RISC-V: add zbb support to string functions Heiko Stuebner
2022-11-13 23:29 ` Conor Dooley
2022-11-13 23:47 ` Heiko Stübner
2022-11-24 22:23 ` Heiko Stübner
2022-11-24 22:32 ` Conor Dooley [this message]
2022-11-24 23:51 ` Heiko Stuebner
2022-11-25 7:49 ` Andrew Jones
2022-11-25 8:17 ` Conor.Dooley
[not found] ` <CAEg0e7h9skbWPVDsz9CdB8dATN5XM9eT-uPY0A7xRZmX=qTU6A@mail.gmail.com>
2022-11-25 15:28 ` Andrew Jones
2022-11-25 16:35 ` Christoph Müllner
2022-11-25 16:39 ` Conor Dooley
2022-11-25 17:02 ` Christoph Müllner
2022-11-25 17:11 ` Conor Dooley
2022-11-25 17:42 ` Christoph Müllner
2022-11-25 16:36 ` Conor Dooley
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=Y3/xGhkbf6qEr+5r@spud \
--to=conor@kernel.org \
--cc=ajones@ventanamicro.com \
--cc=christoph.muellner@vrull.eu \
--cc=emil.renner.berthing@canonical.com \
--cc=heiko@sntech.de \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=philipp.tomsich@vrull.eu \
--cc=prabhakar.csengg@gmail.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 NNTP newsgroup(s).