* [PATCH] RISC-V: Allow ISA subsets to be disabled
@ 2017-03-30 19:46 Palmer Dabbelt
2017-03-31 8:15 ` Alex Bradbury
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Palmer Dabbelt @ 2017-03-30 19:46 UTC (permalink / raw)
To: binutils; +Cc: Alex Bradbury, Palmer Dabbelt
Without this patch, passing "-march=rv64ic -march=rv64i" results in
you getting a "RV64IC" toolchain, which isn't expected.
gas/ChangeLog:
2017-03-30 Palmer Dabbelt <palmer@dabbelt.com>
* config/tc-riscv.c (riscv_clear_subsets): New function.
(riscv_add_subset): Call riscv_clear_subsets and riscv_set_rvc to
clear RVC when it's been previously set.
---
gas/ChangeLog | 6 ++++++
gas/config/tc-riscv.c | 15 +++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/gas/ChangeLog b/gas/ChangeLog
index a5c1d08..6f419f7 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,9 @@
+2017-03-30 Palmer Dabbelt <palmer@dabbelt.com>
+
+ * config/tc-riscv.c (riscv_clear_subsets): New function.
+ (riscv_add_subset): Call riscv_clear_subsets and riscv_set_rvc to
+ clear RVC when it's been previously set.
+
2017-03-30 Pip Cet <pipcet@gmail.com>
* config/tc-wasm32.h: New file: Add WebAssembly assembler target.
diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 68b28f7..2830ba1 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -121,6 +121,18 @@ riscv_subset_supports (const char *feature)
}
static void
+riscv_clear_subsets (void)
+{
+ while (riscv_subsets != NULL)
+ {
+ struct riscv_subset *next = riscv_subsets->next;
+ free (riscv_subsets->name);
+ free (riscv_subsets);
+ riscv_subsets = next;
+ }
+}
+
+static void
riscv_add_subset (const char *subset)
{
struct riscv_subset *s = xmalloc (sizeof *s);
@@ -139,6 +151,8 @@ riscv_set_arch (const char *s)
const char *extension = NULL;
const char *p = s;
+ riscv_clear_subsets();
+
if (strncmp (p, "rv32", 4) == 0)
{
xlen = 32;
@@ -1808,6 +1822,7 @@ riscv_after_parse_args (void)
riscv_set_arch (xlen == 64 ? "rv64g" : "rv32g");
/* Add the RVC extension, regardless of -march, to support .option rvc. */
+ riscv_set_rvc (FALSE);
if (riscv_subset_supports ("c"))
riscv_set_rvc (TRUE);
else
--
2.10.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Allow ISA subsets to be disabled
2017-03-30 19:46 [PATCH] RISC-V: Allow ISA subsets to be disabled Palmer Dabbelt
@ 2017-03-31 8:15 ` Alex Bradbury
2017-03-31 8:39 ` Alex Bradbury
2017-03-31 9:09 ` Nick Clifton
2017-04-01 10:50 ` [BUILDROBOT] RISC-V: error: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type (was: [PATCH] RISC-V: Allow ISA subsets to be disabled) Jan-Benedict Glaw
2 siblings, 1 reply; 7+ messages in thread
From: Alex Bradbury @ 2017-03-31 8:15 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: binutils
On 30 March 2017 at 20:45, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> Without this patch, passing "-march=rv64ic -march=rv64i" results in
> you getting a "RV64IC" toolchain, which isn't expected.
>
> gas/ChangeLog:
>
> 2017-03-30 Palmer Dabbelt <palmer@dabbelt.com>
>
> * config/tc-riscv.c (riscv_clear_subsets): New function.
> (riscv_add_subset): Call riscv_clear_subsets and riscv_set_rvc to
> clear RVC when it's been previously set.
For what it's worth, the current behaviour (i.e. the behaviour before
this patch) seems to be in line with other architectures. e.g. if I do
`gcc -march=native -march=generic` then the latter won't override the
former - at least as evidenced by `gcc -march=native -march=generic -E
-v - </dev/null`.
Best,
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Allow ISA subsets to be disabled
2017-03-31 8:15 ` Alex Bradbury
@ 2017-03-31 8:39 ` Alex Bradbury
2017-03-31 9:23 ` Nick Clifton
0 siblings, 1 reply; 7+ messages in thread
From: Alex Bradbury @ 2017-03-31 8:39 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: binutils
On 31 March 2017 at 09:15, Alex Bradbury <asb@asbradbury.org> wrote:
> On 30 March 2017 at 20:45, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> Without this patch, passing "-march=rv64ic -march=rv64i" results in
>> you getting a "RV64IC" toolchain, which isn't expected.
>>
>> gas/ChangeLog:
>>
>> 2017-03-30 Palmer Dabbelt <palmer@dabbelt.com>
>>
>> * config/tc-riscv.c (riscv_clear_subsets): New function.
>> (riscv_add_subset): Call riscv_clear_subsets and riscv_set_rvc to
>> clear RVC when it's been previously set.
>
> For what it's worth, the current behaviour (i.e. the behaviour before
> this patch) seems to be in line with other architectures. e.g. if I do
> `gcc -march=native -march=generic` then the latter won't override the
> former - at least as evidenced by `gcc -march=native -march=generic -E
> -v - </dev/null`.
My apologies, over-riding behaviour _is_ standard for as which is
obviously the relevant comparison.
$ cat foo.s
foo:
vmulsd %xmm2, %xmm1, %xmm1
vaddsd %xmm0, %xmm1, %xmm0
ret
$ as -march=corei7+avx -march=generic64 foo.s
foo.s: Assembler messages:
foo.s:2: Error: `vmulsd' is not supported on `generic64'
foo.s:3: Error: `vaddsd' is not supported on `generic64'
It's a shame gcc and as are inconsistent in their argument handling in
this way, at least on x86-64. Palmer: are you intending to make a
matching change to -march for gcc, or to stick with the non-overriding
behaviour -march has on x86 (and other platforms?).
Best,
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Allow ISA subsets to be disabled
2017-03-30 19:46 [PATCH] RISC-V: Allow ISA subsets to be disabled Palmer Dabbelt
2017-03-31 8:15 ` Alex Bradbury
@ 2017-03-31 9:09 ` Nick Clifton
2017-04-01 10:50 ` [BUILDROBOT] RISC-V: error: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type (was: [PATCH] RISC-V: Allow ISA subsets to be disabled) Jan-Benedict Glaw
2 siblings, 0 replies; 7+ messages in thread
From: Nick Clifton @ 2017-03-31 9:09 UTC (permalink / raw)
To: Palmer Dabbelt, binutils; +Cc: Alex Bradbury
Hi Palmer,
> 2017-03-30 Palmer Dabbelt <palmer@dabbelt.com>
>
> * config/tc-riscv.c (riscv_clear_subsets): New function.
> (riscv_add_subset): Call riscv_clear_subsets and riscv_set_rvc to
> clear RVC when it's been previously set.
Approved - please apply.
Cheers
Nick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Allow ISA subsets to be disabled
2017-03-31 8:39 ` Alex Bradbury
@ 2017-03-31 9:23 ` Nick Clifton
2017-03-31 16:46 ` Palmer Dabbelt
0 siblings, 1 reply; 7+ messages in thread
From: Nick Clifton @ 2017-03-31 9:23 UTC (permalink / raw)
To: Alex Bradbury, Palmer Dabbelt; +Cc: binutils
Hi Alex,
> It's a shame gcc and as are inconsistent in their argument handling in
> this way, at least on x86-64.
Agreed.
I had always thought that a later option overriding an earlier option was
standard behaviour, but when I checked the gcc documentation I could not
find any specific requirement for this. I was quite surprised. Personally
I think that gcc is wrong. To my mind it is intuitive that a later option
would override an earlier one. If it does not then the tool concerned, at
the very least, ought to issue a warning message that the later option is
being ignored.
So, in summary - I think that this is a gcc bug, not a binutils bug.
Cheers
Nick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] RISC-V: Allow ISA subsets to be disabled
2017-03-31 9:23 ` Nick Clifton
@ 2017-03-31 16:46 ` Palmer Dabbelt
0 siblings, 0 replies; 7+ messages in thread
From: Palmer Dabbelt @ 2017-03-31 16:46 UTC (permalink / raw)
To: Nick Clifton; +Cc: Alex Bradbury, binutils
On Fri, 31 Mar 2017 02:22:53 PDT (-0700), Nick Clifton wrote:
> Hi Alex,
>
>> It's a shame gcc and as are inconsistent in their argument handling in
>> this way, at least on x86-64.
>
> Agreed.
>
> I had always thought that a later option overriding an earlier option was
> standard behaviour, but when I checked the gcc documentation I could not
> find any specific requirement for this. I was quite surprised. Personally
> I think that gcc is wrong. To my mind it is intuitive that a later option
> would override an earlier one. If it does not then the tool concerned, at
> the very least, ought to issue a warning message that the later option is
> being ignored.
>
> So, in summary - I think that this is a gcc bug, not a binutils bug.
I agree (though I guess that's implicit from having posted the patch :)).
We're trying really hard to make sure RISC-V has a sane set of machine-specific
options that are compatible everywhere (binutils, GCC, and LLVM) so we don't
get into this sort of mess in the future.
Committed.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [BUILDROBOT] RISC-V: error: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type (was: [PATCH] RISC-V: Allow ISA subsets to be disabled)
2017-03-30 19:46 [PATCH] RISC-V: Allow ISA subsets to be disabled Palmer Dabbelt
2017-03-31 8:15 ` Alex Bradbury
2017-03-31 9:09 ` Nick Clifton
@ 2017-04-01 10:50 ` Jan-Benedict Glaw
2 siblings, 0 replies; 7+ messages in thread
From: Jan-Benedict Glaw @ 2017-04-01 10:50 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]
Hi Palmer,
On Thu, 2017-03-30 12:45:42 -0700, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> Without this patch, passing "-march=rv64ic -march=rv64i" results in
> you getting a "RV64IC" toolchain, which isn't expected.
>
> gas/ChangeLog:
>
> 2017-03-30 Palmer Dabbelt <palmer@dabbelt.com>
>
> * config/tc-riscv.c (riscv_clear_subsets): New function.
> (riscv_add_subset): Call riscv_clear_subsets and riscv_set_rvc to
> clear RVC when it's been previously set.
Build robot catched a new warning in the build found at
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=700765:
gcc -DHAVE_CONFIG_H -I. -I/home/jbglaw/repos/binutils_gdb/gas -I. -I/home/jbglaw/repos/binutils_gdb/gas -I../bfd -I/home/jbglaw/repos/binutils_gdb/gas/config -I/home/jbglaw/repos/binutils_gdb/gas/../include -I/home/jbglaw/repos/binutils_gdb/gas/.. -I/home/jbglaw/repos/binutils_gdb/gas/../bfd -DLOCALEDIR="\"/home/jbglaw/build/riscv32-unknown-linux-gnu/_install_/share/locale\"" -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Werror -Wwrite-strings -I/home/jbglaw/repos/binutils_gdb/gas/../zlib -g -O2 -MT tc-riscv.o -MD -MP -MF .deps/tc-riscv.Tpo -c -o tc-riscv.o `test -f 'config/tc-riscv.c' || echo '/home/jbglaw/repos/binutils_gdb/gas/'`config/tc-riscv.c
/home/jbglaw/repos/binutils_gdb/gas/config/tc-riscv.c: In function ‘riscv_clear_subsets’:
/home/jbglaw/repos/binutils_gdb/gas/config/tc-riscv.c:129:13: error: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type [-Werror]
free (riscv_subsets->name);
^
In file included from /home/jbglaw/repos/binutils_gdb/gas/as.h:58:0,
from /home/jbglaw/repos/binutils_gdb/gas/config/tc-riscv.c:23:
/usr/include/stdlib.h:483:13: note: expected ‘void *’ but argument is of type ‘const char *’
extern void free (void *__ptr) __THROW;
^
cc1: all warnings being treated as errors
Makefile:1651: recipe for target 'tc-riscv.o' failed
MfG, JBG
--
Jan-Benedict Glaw jbglaw@lug-owl.de +49-172-7608481
Signature of: Lauf nicht vor Deinem Glück davon:
the second : Es könnte hinter Dir stehen!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-04-01 10:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-30 19:46 [PATCH] RISC-V: Allow ISA subsets to be disabled Palmer Dabbelt
2017-03-31 8:15 ` Alex Bradbury
2017-03-31 8:39 ` Alex Bradbury
2017-03-31 9:23 ` Nick Clifton
2017-03-31 16:46 ` Palmer Dabbelt
2017-03-31 9:09 ` Nick Clifton
2017-04-01 10:50 ` [BUILDROBOT] RISC-V: error: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type (was: [PATCH] RISC-V: Allow ISA subsets to be disabled) Jan-Benedict Glaw
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).