From: "Heiko Stübner" <heiko@sntech.de>
To: Conor Dooley <conor@kernel.org>
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 23:23:08 +0100 [thread overview]
Message-ID: <14728581.RDIVbhacDa@diego> (raw)
In-Reply-To: <Y3F936DrR6SMcJvR@spud>
> > 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.
>
> > 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.
>
> > 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, ...)
> > 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 :-) .
> > + /* 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? :-)
Heiko
_______________________________________________
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:23 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 [this message]
2022-11-24 22:32 ` Conor Dooley
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=14728581.RDIVbhacDa@diego \
--to=heiko@sntech.de \
--cc=ajones@ventanamicro.com \
--cc=christoph.muellner@vrull.eu \
--cc=conor@kernel.org \
--cc=emil.renner.berthing@canonical.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.