From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com [IPv6:2a00:1450:4864:20::22b]) by sourceware.org (Postfix) with ESMTPS id 6E3313858C3A for ; Wed, 2 Nov 2022 22:21:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6E3313858C3A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu Received: by mail-lj1-x22b.google.com with SMTP id h12so78767ljg.9 for ; Wed, 02 Nov 2022 15:21:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=yLP4Oi+AOh4oboPq6tzeRp/8OPENZfx0o6LAtckDIdc=; b=Bec3U9ZZOO4OCyqgPecF03gIaqWGwI4WbV1wLoIEF14aCIq9qES+WB6iE8g5HsUgiA uI4nVyC7wo4zcyFfSGWwNTCYVu58ObS15HUR9HSEFIn6pxTWgxjuzEtxxx7Cg0Y0wmgV wXL3lAcbJ3jqYhYacQKTUp37Qmr6zS+6hSW2yiYjgXd23Ho//STIRHrWQoXPx8HJlefW YCT4lZdz3moRjHvHsWeYML2DI0COeCK0g3uNIXwtxb9o2ia0HbsjtT8aws8MrVJOyI8Y sBT6zA9XSDEEmYJG2XiElx69SElLZlX2aVkLynsISdjH+sNpB2t6/trAB2qmNn40/DIA 5S4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=yLP4Oi+AOh4oboPq6tzeRp/8OPENZfx0o6LAtckDIdc=; b=WbPMBcyzHp5++sQ/6qu49r0bHN8xNjVX58nFiFf1iBU4HFOSWi/4IavaITbMdS38kc iIc6ZhTozG5zAtjL87c6OjLtbN9fRyTSzy4vzPvEu15+GP0YL49focraUzCXe5YTnovP WrVg16t2bqAZe3fsDb0EFKq2bFsvVTnMKACwf+ImktVCaUgyyh724tb+/fx6oY6DTC5p c9fu2mGk+v6e5XQzdli+Dxkqu7MDEQXkUwnUnEabX9OAuFYzS/e+oAMwsPXfz/RTTdRl fF2sQJ9gjonmjVkgYB/3HSR31qORr6SgPcuqeQMsiBr6pqddM7Hjmt0Pg8tG15q8pTXJ obMQ== X-Gm-Message-State: ACrzQf0Crlz6t3qreg49L1BTQFKxXFccWyLyWchZTe0Enw40ndTkKJcy O9yIXW3Y4VWb/BcqFoB6vt+YYD7rfbHvFMFU08y3Sw== X-Google-Smtp-Source: AMsMyM40gZJB7WQIEehCs+wRZJpYIyrVu2todhqCugz8vOFbb8EMpC+ZSSxEcrZz+HlvsjUzY2qmXtcjcyTUrfYdqcA= X-Received: by 2002:a2e:8781:0:b0:26d:e758:ce84 with SMTP id n1-20020a2e8781000000b0026de758ce84mr9989397lji.178.1667427668742; Wed, 02 Nov 2022 15:21:08 -0700 (PDT) MIME-Version: 1.0 References: <20221102125235.2325572-2-jiawei@iscas.ac.cn> In-Reply-To: From: Philipp Tomsich Date: Wed, 2 Nov 2022 23:20:57 +0100 Message-ID: Subject: Re: [RFC] RISC-V: Minimal supports for new extensions in profile. To: Palmer Dabbelt Cc: jiawei@iscas.ac.cn, gcc-patches@gcc.gnu.org, wuwei2016@iscas.ac.cn, kito.cheng@sifive.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, 2 Nov 2022 at 23:06, Palmer Dabbelt wrote: > > On Wed, 02 Nov 2022 05:52:34 PDT (-0700), jiawei@iscas.ac.cn wrote: > > This patch just add name support contain in profiles. > > Set the extension version as 0.1. > > Or maybe v0.8, as they're in the v0.8 profile spec? I doubt it really > matters, though. Either way we'll need a -mprofile-spec-version (or > whatever) for these, as these one-phrase definitions will almost > certainly change. > > This also doesn't couple these new extensions to the profiles in any > way. IMO that's a sane thing to do, but they're only defined as part of > the mandatory profile section so I'm just double-checking here > . > > We'll also need news entries and I don't see any testing results, though > those are probably pretty easy here. > > > > > gcc/ChangeLog: > > > > * common/config/riscv/riscv-common.cc: New extensions. > > * config/riscv/riscv-opts.h (MASK_ZICCAMOA): New mask. > > (MASK_ZICCIF): Ditto. > > (MASK_ZICCLSM): Ditto. > > (MASK_ZICCRSE): Ditto. > > (MASK_ZICNTR): Ditto. > > (MASK_ZIHINTPAUSE): Ditto. > > (MASK_ZIHPM): Ditto. > > (TARGET_ZICCAMOA): New target. > > (TARGET_ZICCIF): Ditto. > > (TARGET_ZICCLSM): Ditto. > > (TARGET_ZICCRSE): Ditto. > > (TARGET_ZICNTR): Ditto. > > (TARGET_ZIHINTPAUSE): Ditto. > > (TARGET_ZIHPM): Ditto. > > (MASK_SVPBMT): New mask. > > > > --- > > gcc/common/config/riscv/riscv-common.cc | 20 ++++++++++++++++++++ > > gcc/config/riscv/riscv-opts.h | 15 +++++++++++++++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc > > index d6404a01205..602491c638d 100644 > > --- a/gcc/common/config/riscv/riscv-common.cc > > +++ b/gcc/common/config/riscv/riscv-common.cc > > @@ -163,6 +163,15 @@ static const struct riscv_ext_version riscv_ext_version_table[] = > > {"zifencei", ISA_SPEC_CLASS_20191213, 2, 0}, > > {"zifencei", ISA_SPEC_CLASS_20190608, 2, 0}, > > > > + {"ziccamoa", ISA_SPEC_CLASS_NONE, 0, 1}, > > + {"ziccif", ISA_SPEC_CLASS_NONE, 0, 1}, > > IMO Ziccif should be sufficiently visible in the object that we can > reject running binaries that require that on systems that don't support > it. It's essentially the same as Ztso, we're adding more constraints to > existing instructions. > > > + {"zicclsm", ISA_SPEC_CLASS_NONE, 0, 1}, > > + {"ziccrse", ISA_SPEC_CLASS_NONE, 0, 1}, > > + {"zicntr", ISA_SPEC_CLASS_NONE, 0, 1}, > > As per Andrew's post here > , > Zicntr and Zihpm should be ignored by software. > > I think you could make that compatibility argument for Zicclsm and > Ziccrse as well, but given that the core of the Zicntr/Zihpm argument is > based on userspace not knowing about priv-spec details such as PMAs I'm > guessing it'd go that way too. That said, these are all listed in the > "features available to user-mode execution environments" section. > > > + > > + {"zihintpause", ISA_SPEC_CLASS_NONE, 0, 1}, > > We should probably have a builtin for this, there's a handful of > userspace cpu_relax()-type calls and having something to select the > flavor of pause instruction based on the target seems generally useful. I had originally submitted this in early 2021 (including a builtin), but we never agreed on details (e.g. whether this should be gated, as it is a true hint): https://gcc.gnu.org/pipermail/gcc-patches/2021-January/562936.html Let me know what behavior we want and I'll submit a v2. Philipp. > > + {"zihpm", ISA_SPEC_CLASS_NONE, 0, 1}, > > See above. > > > + > > {"zba", ISA_SPEC_CLASS_NONE, 1, 0}, > > {"zbb", ISA_SPEC_CLASS_NONE, 1, 0}, > > {"zbc", ISA_SPEC_CLASS_NONE, 1, 0}, > > There's some missing ones, just poking through the profile I can find: > Za64rs and Zic64b, but there's a lot in there and I'm kind of getting my > eyes crossed already. > > I'd argue that Za64rs should be handled like Ziccif, but we don't have a > lot of bits left in the header. I just sent some patches to the ELF > psABI spec: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/351 > > > @@ -219,6 +228,7 @@ static const struct riscv_ext_version riscv_ext_version_table[] = > > > > {"svinval", ISA_SPEC_CLASS_NONE, 1, 0}, > > {"svnapot", ISA_SPEC_CLASS_NONE, 1, 0}, > > + {"svpbmt", ISA_SPEC_CLASS_NONE, 0, 1}, > > > > /* Terminate the list. */ > > {NULL, ISA_SPEC_CLASS_NONE, 0, 0} > > @@ -1179,6 +1189,14 @@ static const riscv_ext_flag_table_t riscv_ext_flag_table[] = > > > > {"zicsr", &gcc_options::x_riscv_zi_subext, MASK_ZICSR}, > > {"zifencei", &gcc_options::x_riscv_zi_subext, MASK_ZIFENCEI}, > > + {"ziccamoa", &gcc_options::x_riscv_zi_subext, MASK_ZICCAMOA}, > > + {"ziccif", &gcc_options::x_riscv_zi_subext, MASK_ZICCIF}, > > + {"zicclsm", &gcc_options::x_riscv_zi_subext, MASK_ZICCLSM}, > > + {"ziccrse", &gcc_options::x_riscv_zi_subext, MASK_ZICCRSE}, > > + {"zicntr", &gcc_options::x_riscv_zi_subext, MASK_ZICNTR}, > > + > > + {"zihintpause", &gcc_options::x_riscv_zi_subext, MASK_ZIHINTPAUSE}, > > + {"zihpm", &gcc_options::x_riscv_zi_subext, MASK_ZIHPM}, > > > > {"zba", &gcc_options::x_riscv_zb_subext, MASK_ZBA}, > > {"zbb", &gcc_options::x_riscv_zb_subext, MASK_ZBB}, > > @@ -1230,6 +1248,7 @@ static const riscv_ext_flag_table_t riscv_ext_flag_table[] = > > {"zvl1024b", &gcc_options::x_riscv_zvl_flags, MASK_ZVL1024B}, > > {"zvl2048b", &gcc_options::x_riscv_zvl_flags, MASK_ZVL2048B}, > > {"zvl4096b", &gcc_options::x_riscv_zvl_flags, MASK_ZVL4096B}, > > + > > Looks like a spurious newline got inserted? If there's a reason for it > then it's best to do this as its own commit. > > > {"zvl8192b", &gcc_options::x_riscv_zvl_flags, MASK_ZVL8192B}, > > {"zvl16384b", &gcc_options::x_riscv_zvl_flags, MASK_ZVL16384B}, > > {"zvl32768b", &gcc_options::x_riscv_zvl_flags, MASK_ZVL32768B}, > > @@ -1242,6 +1261,7 @@ static const riscv_ext_flag_table_t riscv_ext_flag_table[] = > > > > {"svinval", &gcc_options::x_riscv_sv_subext, MASK_SVINVAL}, > > {"svnapot", &gcc_options::x_riscv_sv_subext, MASK_SVNAPOT}, > > + {"svpbmt", &gcc_options::x_riscv_sv_subext, MASK_SVPBMT}, > > Not technically part of the profiles so we could split it out, but I > don't think it's strictly necessary. > > > {NULL, NULL, 0} > > }; > > diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h > > index 1dfe8c89209..906b6280188 100644 > > --- a/gcc/config/riscv/riscv-opts.h > > +++ b/gcc/config/riscv/riscv-opts.h > > @@ -69,9 +69,23 @@ enum stack_protector_guard { > > > > #define MASK_ZICSR (1 << 0) > > #define MASK_ZIFENCEI (1 << 1) > > +#define MASK_ZICCAMOA (1 << 2) > > +#define MASK_ZICCIF (1 << 3) > > +#define MASK_ZICCLSM (1 << 4) > > +#define MASK_ZICCRSE (1 << 5) > > +#define MASK_ZICNTR (1 << 6) > > +#define MASK_ZIHINTPAUSE (1 << 7) > > +#define MASK_ZIHPM (1 << 8) > > > > #define TARGET_ZICSR ((riscv_zi_subext & MASK_ZICSR) != 0) > > #define TARGET_ZIFENCEI ((riscv_zi_subext & MASK_ZIFENCEI) != 0) > > +#define TARGET_ZICCAMOA ((riscv_zi_subext & MASK_ZICCAMOA) != 0) > > +#define TARGET_ZICCIF ((riscv_zi_subext & MASK_ZICCIF) != 0) > > +#define TARGET_ZICCLSM ((riscv_zi_subext & MASK_ZICCLSM) != 0) > > +#define TARGET_ZICCRSE ((riscv_zi_subext & MASK_ZICCRSE) != 0) > > +#define TARGET_ZICNTR ((riscv_zi_subext & MASK_ZICNTR) != 0) > > +#define TARGET_ZIHINTPAUSE ((riscv_zi_subext & MASK_ZIHINTPAUSE) != 0) > > +#define TARGET_ZIHPM ((riscv_zi_subext & MASK_ZIHPM) != 0) > > > > #define MASK_ZBA (1 << 0) > > #define MASK_ZBB (1 << 1) > > @@ -174,6 +188,7 @@ enum stack_protector_guard { > > > > #define MASK_SVINVAL (1 << 0) > > #define MASK_SVNAPOT (1 << 1) > > +#define MASK_SVPBMT (1 << 2) > > > > #define TARGET_SVINVAL ((riscv_sv_subext & MASK_SVINVAL) != 0) > > #define TARGET_SVNAPOT ((riscv_sv_subext & MASK_SVNAPOT) != 0)