[PATCH v1 0/6] RISC-V -march handling improvements

[PATCH v1 0/6] RISC-V -march handling improvements

[Palmer Dabbelt]

As pointed out recently [1], sparse is parsing -march on RISC-V in order
to obtain the default set of preprocessor macros to use.  Back when this
was written ISA string was a simple affair, but these days it's a lot
more complicated.  It's going to be a big chunk of work to get a proper
ISA string parser into sparse, but we can at least fix the breakages for
the subset of legal ISA strings that Linux currently uses (and are
breaking users).

This patch set does three things:

* Stops die()ing on unknown ISA strings, unless the user has passed
  -Wsparse-error.  This prints a warning and guesses at the macros to
  use, which is probably fine for Linux.
* Cleans up some of the differences between GCC's -march parsing and
  sparse's.  None of this should really matter for Linux, as GCC will
  blow up on bad ISA strings, but it just seemed worth doing when I was
  in there.
* Adds support for the Zicsr and Zifencei extensions, which were
  recently enabled.  With these the unknown ISA string warning goes away
  for Linux builds.

They're all sort of independent (and happen in this order), but they're
all touching the same code so I'm just sending it as a series.  It's my
first time touching sparse.

I've poked around with the first patch on its own and it seems to
largely work as expected: I'm still getting a bunch of sparse-related
warnings when I turn on sparse in my builds, but at least I don't get an
error (after updating to a binutils that supports the new arguments, so
Linux detects them).  I tried CF="-Wsparse-error", which also behaves as
expected (that trinary boolean tripped me up for a bit).

The first patch alone should be a sufficient band-aid for systems that
are actively broken right now, the rest are cleanups -- these may be
necessary to get the RISC-V port sparse-clean, but that's a WIP so there
might be more.  I'm going to play around with that, but just looking at
the volume of spew it's probably going to take a while.  I gave these
patches a bit of testing one-by-one, but not nearly as much as the
first.

I just spun up a sparse repo [2] at kernel.org, these are on the riscv
branch if that helps for anyone.  I've also started messing around with
parsing a few more of the multi-letter extensions, but there's so much
coupling I got fed up -- it's on riscv-wip, but I definitely don't like
that last patch.  I figured it's better to send out these bits, as they
look solid to me and builds are broken.  The new stuff (B, K, and V) are
all in GCC-12 anyway, so we have a bit of time before they're useful.

[1]: https://lore.kernel.org/linux-sparse/mhng-c280d48c-477d-4589-baee-255c774b5a51@palmer-mbp2014/T/#maef705f448e4a1f12d853c0d8bc756f037ce1ce0
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/palmer/sparse.git

[PATCH v1 1/6] RISC-V: Respect -Wsparse-error for -march errors

[Palmer Dabbelt]

Parsing RISC-V ISA strings is extremely complicated: there are many
extensions, versions of extensions, versions of the ISA string rules,
and a bunch of unwritten rules to deal with all the bugs that fell out
of that complexity.

Rather than forcing users to see an error when the ISA string parsing
fails, just stop parsing where we get lost.  Changes tend to end up at
the end of the ISA string, so that's probably going to work (and if
it doesn't there's a warning to true and clue folks in).

This does have the oddity in that "-Wsparse-error -march=..." behaves
differently than "-march... -Wsparse-error", but that's already the case
for "--arch=... -march=..." and "-march=... --arch=...".  Both
"-Wsparse-error" and "--arch" are sparse-specific arguments, so they're
probably both going to be in the same place.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 lib.h          |  5 +++++
 options.c      |  6 ------
 target-riscv.c | 16 ++++++++++++++--
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/lib.h b/lib.h
index b96e3192..2c0d7116 100644
--- a/lib.h
+++ b/lib.h
@@ -125,6 +125,11 @@ enum phase {
 #define	PASS_OPTIM		(1UL << PASS__OPTIM)
 #define	PASS_FINAL		(1UL << PASS__FINAL)
 
+enum flag_type {
+	FLAG_OFF,
+	FLAG_ON,
+	FLAG_FORCE_OFF,
+};
 
 extern void add_pre_buffer(const char *fmt, ...) FORMAT_ATTR(1);
 extern void predefine(const char *name, int weak, const char *fmt, ...) FORMAT_ATTR(3);
diff --git a/options.c b/options.c
index 6704fc8d..41a98240 100644
--- a/options.c
+++ b/options.c
@@ -23,12 +23,6 @@
 # define __GNUC_PATCHLEVEL__ 0
 #endif
 
-enum flag_type {
-	FLAG_OFF,
-	FLAG_ON,
-	FLAG_FORCE_OFF
-};
-
 int die_if_error = 0;
 int do_output = 1;
 int gcc_major = __GNUC__;
diff --git a/target-riscv.c b/target-riscv.c
index 6d9113c1..f5cc6cc3 100644
--- a/target-riscv.c
+++ b/target-riscv.c
@@ -3,6 +3,7 @@
 #include "target.h"
 #include "machine.h"
 #include <string.h>
+#include <stdio.h>
 
 #define RISCV_32BIT	(1 << 0)
 #define RISCV_64BIT	(1 << 1)
@@ -60,7 +61,18 @@ static void parse_march_riscv(const char *arg)
 			goto ext;
 		}
 	}
-	die("invalid argument to '-march': '%s'\n", arg);
+
+unknown:
+	/*
+	 * This behaves like do_warn() / do_error(), but we don't have a
+	 * position so it's just inline here.
+	 */
+	fflush(stdout);
+	fprintf(stderr, "%s: invalid argument to '-march': '%s'\n",
+		Wsparse_error == FLAG_ON ? "error" : "warning", arg);
+	if (Wsparse_error == FLAG_ON)
+		has_error |= ERROR_CURR_PHASE;
+	return;
 
 ext:
 	for (i = 0; i < ARRAY_SIZE(extensions); i++) {
@@ -73,7 +85,7 @@ ext:
 		}
 	}
 	if (arg[0])
-		die("invalid argument to '-march': '%s'\n", arg);
+		goto unknown;
 }
 
 static void init_riscv(const struct target *self)
-- 
2.34.1

[PATCH v1 2/6] RISC-V: Match GCC's semantics for multiple -march instances

[Palmer Dabbelt]

GCC's semantics for "-march=X -march=Y" are that Y entirely overrides X,
but sparse takes the union of these two ISA strings.  This fixes the
behavior by setting, instead of oring, the flags whenever a base ISA is
encountered.  RISC-V ISA strings can only have a single base ISA, it's
not like x86 where the 64-bit ISA is an extension of the 32-bit ISA.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 target-riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-riscv.c b/target-riscv.c
index f5cc6cc3..494c08db 100644
--- a/target-riscv.c
+++ b/target-riscv.c
@@ -56,7 +56,7 @@ static void parse_march_riscv(const char *arg)
 		size_t len = strlen(pat);
 
 		if (!strncmp(arg, pat, len)) {
-			riscv_flags |= basic_sets[i].flags;
+			riscv_flags = basic_sets[i].flags;
 			arg += len;
 			goto ext;
 		}
-- 
2.34.1

[PATCH v1 3/6] RISC-V: Remove the unimplemented ISA extensions

[Palmer Dabbelt]

This made sense when we die()d on unknown ISA extensions, but now that
we're just warning it's actually a bit detrimental: users won't see that
their unimplemented ISA extensions are silently having the wrong
definitions set, which may cause hard to debug failures.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 target-riscv.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/target-riscv.c b/target-riscv.c
index 494c08db..924259af 100644
--- a/target-riscv.c
+++ b/target-riscv.c
@@ -37,17 +37,7 @@ static void parse_march_riscv(const char *arg)
 		{ "f",		RISCV_FLOAT|RISCV_FDIV },
 		{ "d",		RISCV_DOUBLE|RISCV_FDIV },
 		{ "g",		RISCV_GENERIC },
-		{ "q",		0 },
-		{ "l",		0 },
 		{ "c",		RISCV_COMP },
-		{ "b",		0 },
-		{ "j",		0 },
-		{ "t",		0 },
-		{ "p",		0 },
-		{ "v",		0 },
-		{ "n",		0 },
-		{ "h",		0 },
-		{ "s",		0 },
 	};
 	int i;
 
-- 
2.34.1

[PATCH v1 4/6] RISC-V: Remove "g" from the extension list

[Palmer Dabbelt]

"g" goes along with the base ISA, but it was being treated as an
extension.  This allows for all sorts of odd ISA strings to be accepted
by sparse, things like "rv32ig" or "rv32gg".  We're still allowing
some oddities, like "rv32ga", but this one was easy to catch.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 target-riscv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target-riscv.c b/target-riscv.c
index 924259af..5076bbaf 100644
--- a/target-riscv.c
+++ b/target-riscv.c
@@ -36,7 +36,6 @@ static void parse_march_riscv(const char *arg)
 		{ "a",		RISCV_ATOMIC },
 		{ "f",		RISCV_FLOAT|RISCV_FDIV },
 		{ "d",		RISCV_DOUBLE|RISCV_FDIV },
-		{ "g",		RISCV_GENERIC },
 		{ "c",		RISCV_COMP },
 	};
 	int i;
-- 
2.34.1

[PATCH v1 5/6] RISC-V: Add the Zicsr extension

[Palmer Dabbelt]

Recent versions of binutils default to an ISA spec version that doesn't
include Zicsr as part of I, so Linux has recently started passing this
in -march.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 target-riscv.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target-riscv.c b/target-riscv.c
index 5076bbaf..afd6fafa 100644
--- a/target-riscv.c
+++ b/target-riscv.c
@@ -17,6 +17,7 @@
 #define RISCV_EMBD	(1 << 9)
 #define RISCV_FPU	(RISCV_FLOAT|RISCV_DOUBLE|RISCV_FDIV)
 #define RISCV_GENERIC	(RISCV_MUL|RISCV_DIV|RISCV_ATOMIC|RISCV_FPU)
+#define RISCV_ZICSR	(1 << 10)
 
 static unsigned int riscv_flags;
 
@@ -34,9 +35,10 @@ static void parse_march_riscv(const char *arg)
 	}, extensions[] = {
 		{ "m",		RISCV_MUL|RISCV_DIV },
 		{ "a",		RISCV_ATOMIC },
-		{ "f",		RISCV_FLOAT|RISCV_FDIV },
-		{ "d",		RISCV_DOUBLE|RISCV_FDIV },
+		{ "f",		RISCV_FLOAT|RISCV_FDIV|RISCV_ZICSR },
+		{ "d",		RISCV_DOUBLE|RISCV_FDIV|RISCV_ZICSR },
 		{ "c",		RISCV_COMP },
+		{ "_zicsr",	RISCV_ZICSR },
 	};
 	int i;
 
@@ -128,6 +130,8 @@ static void predefine_riscv(const struct target *self)
 		predefine("__riscv_mul", 1, "1");
 	if ((riscv_flags & RISCV_MUL) && (riscv_flags & RISCV_DIV))
 		predefine("__riscv_muldiv", 1, "1");
+	if (riscv_flags & RISCV_ZICSR)
+		predefine("__riscv_zicsr", 1, "1");
 
 	if (cmodel)
 		predefine_strong("__riscv_cmodel_%s", cmodel);
-- 
2.34.1

[PATCH v1 6/6] RISC-V: Add the Zifencei extension

[Palmer Dabbelt]

Recent versions of binutils default to an ISA spec version that doesn't
include Zifencei as part of I, so Linux has recently started passing
this in -march.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 target-riscv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-riscv.c b/target-riscv.c
index afd6fafa..ff4dfba3 100644
--- a/target-riscv.c
+++ b/target-riscv.c
@@ -18,6 +18,7 @@
 #define RISCV_FPU	(RISCV_FLOAT|RISCV_DOUBLE|RISCV_FDIV)
 #define RISCV_GENERIC	(RISCV_MUL|RISCV_DIV|RISCV_ATOMIC|RISCV_FPU)
 #define RISCV_ZICSR	(1 << 10)
+#define RISCV_ZIFENCEI	(1 << 11)
 
 static unsigned int riscv_flags;
 
@@ -39,6 +40,7 @@ static void parse_march_riscv(const char *arg)
 		{ "d",		RISCV_DOUBLE|RISCV_FDIV|RISCV_ZICSR },
 		{ "c",		RISCV_COMP },
 		{ "_zicsr",	RISCV_ZICSR },
+		{ "_zifencei",	RISCV_ZIFENCEI },
 	};
 	int i;
 
@@ -132,6 +134,8 @@ static void predefine_riscv(const struct target *self)
 		predefine("__riscv_muldiv", 1, "1");
 	if (riscv_flags & RISCV_ZICSR)
 		predefine("__riscv_zicsr", 1, "1");
+	if (riscv_flags & RISCV_ZIFENCEI)
+		predefine("__riscv_zifencei", 1, "1");
 
 	if (cmodel)
 		predefine_strong("__riscv_cmodel_%s", cmodel);
-- 
2.34.1

Re: [PATCH v1 0/6] RISC-V -march handling improvements

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3399 bytes --]

On 01.04.2022 22:00:35, Palmer Dabbelt wrote:
> As pointed out recently [1], sparse is parsing -march on RISC-V in order
> to obtain the default set of preprocessor macros to use.  Back when this
> was written ISA string was a simple affair, but these days it's a lot
> more complicated.  It's going to be a big chunk of work to get a proper
> ISA string parser into sparse, but we can at least fix the breakages for
> the subset of legal ISA strings that Linux currently uses (and are
> breaking users).
> 
> This patch set does three things:
> 
> * Stops die()ing on unknown ISA strings, unless the user has passed
>   -Wsparse-error.  This prints a warning and guesses at the macros to
>   use, which is probably fine for Linux.
> * Cleans up some of the differences between GCC's -march parsing and
>   sparse's.  None of this should really matter for Linux, as GCC will
>   blow up on bad ISA strings, but it just seemed worth doing when I was
>   in there.
> * Adds support for the Zicsr and Zifencei extensions, which were
>   recently enabled.  With these the unknown ISA string warning goes away
>   for Linux builds.
> 
> They're all sort of independent (and happen in this order), but they're
> all touching the same code so I'm just sending it as a series.  It's my
> first time touching sparse.
> 
> I've poked around with the first patch on its own and it seems to
> largely work as expected: I'm still getting a bunch of sparse-related
> warnings when I turn on sparse in my builds, but at least I don't get an
> error (after updating to a binutils that supports the new arguments, so
> Linux detects them).  I tried CF="-Wsparse-error", which also behaves as
> expected (that trinary boolean tripped me up for a bit).
> 
> The first patch alone should be a sufficient band-aid for systems that
> are actively broken right now, the rest are cleanups -- these may be
> necessary to get the RISC-V port sparse-clean, but that's a WIP so there
> might be more.  I'm going to play around with that, but just looking at
> the volume of spew it's probably going to take a while.  I gave these
> patches a bit of testing one-by-one, but not nearly as much as the
> first.
> 
> I just spun up a sparse repo [2] at kernel.org, these are on the riscv
> branch if that helps for anyone.  I've also started messing around with
> parsing a few more of the multi-letter extensions, but there's so much
> coupling I got fed up -- it's on riscv-wip, but I definitely don't like
> that last patch.  I figured it's better to send out these bits, as they
> look solid to me and builds are broken.  The new stuff (B, K, and V) are
> all in GCC-12 anyway, so we have a bit of time before they're useful.
> 
> [1]: https://lore.kernel.org/linux-sparse/mhng-c280d48c-477d-4589-baee-255c774b5a51@palmer-mbp2014/T/#maef705f448e4a1f12d853c0d8bc756f037ce1ce0
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/palmer/sparse.git

Works without warnings on Debian testing, with gcc-riscv64-linux-gnu
4:11.2.0--1.

Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/6] RISC-V -march handling improvements

[Palmer Dabbelt]

On Mon, 04 Apr 2022 10:52:53 PDT (-0700), mkl@pengutronix.de wrote:
> On 01.04.2022 22:00:35, Palmer Dabbelt wrote:
>> As pointed out recently [1], sparse is parsing -march on RISC-V in order
>> to obtain the default set of preprocessor macros to use.  Back when this
>> was written ISA string was a simple affair, but these days it's a lot
>> more complicated.  It's going to be a big chunk of work to get a proper
>> ISA string parser into sparse, but we can at least fix the breakages for
>> the subset of legal ISA strings that Linux currently uses (and are
>> breaking users).
>> 
>> This patch set does three things:
>> 
>> * Stops die()ing on unknown ISA strings, unless the user has passed
>>   -Wsparse-error.  This prints a warning and guesses at the macros to
>>   use, which is probably fine for Linux.
>> * Cleans up some of the differences between GCC's -march parsing and
>>   sparse's.  None of this should really matter for Linux, as GCC will
>>   blow up on bad ISA strings, but it just seemed worth doing when I was
>>   in there.
>> * Adds support for the Zicsr and Zifencei extensions, which were
>>   recently enabled.  With these the unknown ISA string warning goes away
>>   for Linux builds.
>> 
>> They're all sort of independent (and happen in this order), but they're
>> all touching the same code so I'm just sending it as a series.  It's my
>> first time touching sparse.
>> 
>> I've poked around with the first patch on its own and it seems to
>> largely work as expected: I'm still getting a bunch of sparse-related
>> warnings when I turn on sparse in my builds, but at least I don't get an
>> error (after updating to a binutils that supports the new arguments, so
>> Linux detects them).  I tried CF="-Wsparse-error", which also behaves as
>> expected (that trinary boolean tripped me up for a bit).
>> 
>> The first patch alone should be a sufficient band-aid for systems that
>> are actively broken right now, the rest are cleanups -- these may be
>> necessary to get the RISC-V port sparse-clean, but that's a WIP so there
>> might be more.  I'm going to play around with that, but just looking at
>> the volume of spew it's probably going to take a while.  I gave these
>> patches a bit of testing one-by-one, but not nearly as much as the
>> first.
>> 
>> I just spun up a sparse repo [2] at kernel.org, these are on the riscv
>> branch if that helps for anyone.  I've also started messing around with
>> parsing a few more of the multi-letter extensions, but there's so much
>> coupling I got fed up -- it's on riscv-wip, but I definitely don't like
>> that last patch.  I figured it's better to send out these bits, as they
>> look solid to me and builds are broken.  The new stuff (B, K, and V) are
>> all in GCC-12 anyway, so we have a bit of time before they're useful.
>> 
>> [1]: https://lore.kernel.org/linux-sparse/mhng-c280d48c-477d-4589-baee-255c774b5a51@palmer-mbp2014/T/#maef705f448e4a1f12d853c0d8bc756f037ce1ce0
>> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/palmer/sparse.git
>
> Works without warnings on Debian testing, with gcc-riscv64-linux-gnu
> 4:11.2.0--1.
>
> Tested-by: Marc Kleine-Budde <mkl@pengutronix.de>

Thanks.  IIRC you were actually getting the failures from the other 
thread, so I think this is OK, but jus for everyone else:

Unfortunately there's a few more variables than just the GCC version, 
this depends on the binutils version (and IIUC the binutils version GCC 
was compiled with) and how Linux was configured.  I was testing with a 
V=1 build to make sure "-march=rv64imafdc_zicsr_zifence" showed up, it 
should be the same for all files so I was just poking one.

>
> regards,
> Marc
>
> -- 
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://www.pengutronix.de  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Re: [PATCH v1 0/6] RISC-V -march handling improvements

[Luc Van Oostenryck]

On Fri, Apr 01, 2022 at 10:00:35PM -0700, Palmer Dabbelt wrote:

Hi,

Thank you for these patches and please accept my apologies
for this much delayed reply.


I've just pushed the Zicsr and Zifencei patches since these solve
an immediate problem and I've some minor questions/objections for
the other ones.

-- Luc

Re: [PATCH v1 1/6] RISC-V: Respect -Wsparse-error for -march errors

[Luc Van Oostenryck]

On Fri, Apr 01, 2022 at 10:00:36PM -0700, Palmer Dabbelt wrote:
> Parsing RISC-V ISA strings is extremely complicated: there are many
> extensions, versions of extensions, versions of the ISA string rules,
> and a bunch of unwritten rules to deal with all the bugs that fell out
> of that complexity.
> 
> Rather than forcing users to see an error when the ISA string parsing
> fails, just stop parsing where we get lost.  Changes tend to end up at
> the end of the ISA string, so that's probably going to work (and if
> it doesn't there's a warning to true and clue folks in).
> 
> This does have the oddity in that "-Wsparse-error -march=..." behaves
> differently than "-march... -Wsparse-error", but that's already the case
> for "--arch=... -march=..." and "-march=... --arch=...".  Both
> "-Wsparse-error" and "--arch" are sparse-specific arguments, so they're
> probably both going to be in the same place.
> 
> diff --git a/target-riscv.c b/target-riscv.c
> index 6d9113c1..f5cc6cc3 100644
> --- a/target-riscv.c
> +++ b/target-riscv.c
> @@ -60,7 +61,18 @@ static void parse_march_riscv(const char *arg)
>  			goto ext;
>  		}
>  	}
> -	die("invalid argument to '-march': '%s'\n", arg);
> +
> +unknown:
> +	/*
> +	 * This behaves like do_warn() / do_error(), but we don't have a
> +	 * position so it's just inline here.
> +	 */
> +	fflush(stdout);
> +	fprintf(stderr, "%s: invalid argument to '-march': '%s'\n",
> +		Wsparse_error == FLAG_ON ? "error" : "warning", arg);
> +	if (Wsparse_error == FLAG_ON)
> +		has_error |= ERROR_CURR_PHASE;
> +	return;

I don't like this because:
1) it's way too much intimate with the way options are parsed
   (enum flag_type should stay local to options.c).
2) -Wsparse-error is a kind of hack to ignore -Werror but keep
   a way to invoke its semantic (but I don' think anyone is using it).
3) I don't think -Wsparse-error (or GCC's -Werror) should be concerned
   with the parsing of options.

I think it would be fine, for now, to always simply report a warning,
like Linus' patch (but I would prefer to just handle the correct parsing).
If reporting an error is important, then I would be happy to jut move
this code into an helper defined in "options.c".

Best regards,
-- Luc

Re: [PATCH v1 2/6] RISC-V: Match GCC's semantics for multiple -march instances

[Luc Van Oostenryck]

On Fri, Apr 01, 2022 at 10:00:37PM -0700, Palmer Dabbelt wrote:
> GCC's semantics for "-march=X -march=Y" are that Y entirely overrides X,
> but sparse takes the union of these two ISA strings.

OK, but then I prefer to explicitly clear riscv_flags at the start
of the function, like:

diff --git a/target-riscv.c b/target-riscv.c
index 3bba7c15ff1f..ca38d76e6465 100644
--- a/target-riscv.c
+++ b/target-riscv.c
@@ -54,6 +54,9 @@ static void parse_march_riscv(const char *arg)
 	};
 	int i;
 
+	// Each -march=.. options entirely overrides previous ones
+	riscv_flags = 0;
+
 	for (i = 0; i < ARRAY_SIZE(basic_sets); i++) {
 		const char *pat = basic_sets[i].pattern;
 		size_t len = strlen(pat);


-- Luc

Re: [PATCH v1 3/6] RISC-V: Remove the unimplemented ISA extensions

[Luc Van Oostenryck]

On Fri, Apr 01, 2022 at 10:00:38PM -0700, Palmer Dabbelt wrote:
> This made sense when we die()d on unknown ISA extensions, but now that
> we're just warning it's actually a bit detrimental: users won't see that
> their unimplemented ISA extensions are silently having the wrong
> definitions set, which may cause hard to debug failures.
> 
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> ---
>  target-riscv.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/target-riscv.c b/target-riscv.c
> index 494c08db..924259af 100644
> --- a/target-riscv.c
> +++ b/target-riscv.c
> @@ -37,17 +37,7 @@ static void parse_march_riscv(const char *arg)
>  		{ "f",		RISCV_FLOAT|RISCV_FDIV },
>  		{ "d",		RISCV_DOUBLE|RISCV_FDIV },
>  		{ "g",		RISCV_GENERIC },
> -		{ "q",		0 },
> -		{ "l",		0 },
>  		{ "c",		RISCV_COMP },
> -		{ "b",		0 },
> -		{ "j",		0 },
> -		{ "t",		0 },
> -		{ "p",		0 },
> -		{ "v",		0 },
> -		{ "n",		0 },
> -		{ "h",		0 },
> -		{ "s",		0 },

OK, it seems than most of them have anyway no chances to be officialized
anytime soon. Maybe just add the define for p & v together with
the Zb* ones when switching __riscv_arch_test).

-- Luc

Re: [PATCH v1 0/6] RISC-V -march handling improvements

[Palmer Dabbelt]

On Sat, 21 May 2022 14:23:25 PDT (-0700), luc.vanoostenryck@gmail.com wrote:
> On Fri, Apr 01, 2022 at 10:00:35PM -0700, Palmer Dabbelt wrote:
>
> Hi,
>
> Thank you for these patches and please accept my apologies
> for this much delayed reply.

No worries, looks like we're about equally slow ;)

> I've just pushed the Zicsr and Zifencei patches since these solve
> an immediate problem and I've some minor questions/objections for
> the other ones.

From looking at sparse/master, it looks like a lot of those comments got 
resolved and merged.  That all looks good to me, I pointed my local 
sparse over to ce1a6720 ("Merge branches 'unreplaced' and 'inline'") and 
will be using that (via `make ... C=1 CF="-Wno-sparse-error"`) in my 
pre-merge testing until something changes.

Thanks!