public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] RISC-V: Add 'Zicntr' and 'Zihpm' support with compatibility measures
@ 2023-08-08  3:17 Tsukasa OI
  2023-08-08  3:17 ` [RFC PATCH 1/2] RISC-V: Base for complex extension implications Tsukasa OI
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Tsukasa OI @ 2023-08-08  3:17 UTC (permalink / raw)
  To: Tsukasa OI, Palmer Dabbelt, Andrew Waterman, Jim Wilson,
	Nelson Chu, Kito Cheng
  Cc: binutils

Hi,

The role of this patch set is very simple: implement recently ratified
'Zicntr' (basic counters and timers) and 'Zihpm' (hardware performance
counters) extensions, formerly a part of the 'I' extension, version 2.0.

Since some draft extensions depend on those extensions, implementing counter
extensions (as a part of data structure) is becoming mandatory.

However, we need to be *very* careful to the implementation.  Because
CSRs and pseudoinstructions for those extensions were a part of 'I' and
more importantly, the first version which separated counters from the 'I'
extension, did not give counters extension names.  So, we needed to
implement such CSRs and pseudoinstructions as a part of 'I' (continuously).


Not breaking the compatibility here is vital (the only exception might be
the new ratified ISA after version 20191213).  So, I implemented those
extensions not to break anything as possible.

The basic idea is, an extension (riscv_subset_t) with an unknown version is
not emitted to an object file (ELF attributes, mapping symbols etc...).

The default mode for existing ISAs is the compatibility mode (Option 1).


[Option 1: Compatibility Mode]

In the compatibility mode (as default),

1.  'I' implies 'Zicntr' and 'Zihpm'.
2.  'Zicntr' and 'Zihpm' DO NOT imply 'Zicsr'.
3.  'Zicntr' and 'Zihpm' don't have version information.

(2.) is the point.  The ratified document says 'Zicntr' and 'Zihpm' depend
on the 'Zicsr' extension but if we do it unconditionally, that would mean
that the 'I' extension indirectly depends on 'Zicsr' (because of 1.), making
a difference from the ratified 'I' extension version 2.1.  In order to keep
the compatibility, making 'Zicntr' and 'Zihpm' against the documentation was
(sadly) necessary.

In the compatibility mode, code like:

> riscv_subset_supports(&rps, "zicntr")

will return true.  Because, even that the version information is missing,
the 'Zicntr' extension exists in the riscv_subset_list_t.  But an extension
with no version means, it will not be a part of the architectural string
emitted as a part of an object file.


[Option 2: Compliant Mode]

We can continue this forever but we have another option.  Break false
dependency when a new ISA (with its version) is ratified and in that time,
require those extensions separately like -march=..._zicntr_zihpm.

In the compliant mode:

1.  'I' DOES NOT imply 'Zicntr' and 'Zihpm'.
2.  'Zicntr' and 'Zihpm' DO imply 'Zicsr'.
3.  'Zicntr' and 'Zihpm' have its version information
    (ratified version 2.0 or possibly a later version)

Note that (1.) and (2.) are very opposite from the compatibility mode.

In this mode, it is compliant to the specification completely.




[Implementing an Option on future ISAs]

Assume that we have a new ISA specification class, ISA_SPEC_CLASS_2024XXXX.
We can choose which option to implement as follows.


[Option 1: Compatibility Mode]

1. Change the contents of check_implicit_compat_counter_from_i.

>  /* Old.  */
>  return *rps->isa_spec <= ISA_SPEC_CLASS_20191213;

>  /* New.  */
>  return *rps->isa_spec <= ISA_SPEC_CLASS_2024XXXX;

Note that the reason we are looking for the ISA specification (instead of
the version of the 'I' extension) is, the 'I' extension version 2.1 is
unlikely to change even when the new ISA specification is ratified.
I assume that two behaviors (option 1 and 2) share the same 'I' version 2.1.

If the version of the 'I' extension changes (even if no *actual* changes
are made) in the new unprivileged ISA version, that would be a bit simpler.

2. Add following entries to riscv_supported_std_z_ext.

Make sure that they don't have the version number.

>  /* ...  */
>  {"zicntr",		ISA_SPEC_CLASS_2024XXXX,	RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION,  0 }, /* Compat.  */
>  /* ...  */
>  {"zihpm",		ISA_SPEC_CLASS_2024XXXX,	RISCV_UNKNOWN_VERSION, RISCV_UNKNOWN_VERSION,  0 }, /* Compat.  */


[Option 2: Compliant Mode]

1. DO NOT change the contents of check_implicit_compat_counter_from_i.
2. Add following entries to riscv_supported_std_z_ext.

Make sure that they DO have the version number.

>  /* ...  */
>  {"zicntr",		ISA_SPEC_CLASS_2024XXXX,	2, 0,  0 },
>  /* ...  */
>  {"zihpm",		ISA_SPEC_CLASS_2024XXXX,	2, 0,  0 },

...and for compatibility, we need to slightly modify riscv-dis.c.  The
disassembler defaults to the latest non-draft ISA and there's currently no
ways to change it.  We need to make changes to riscv-dis.c by either:

1.  Entering special compatibility mode (even on the latest ISA, enable
    'Zicntr' extension ['Zihpm' is not necessary on the disassembler]) or
2.  Enabling to change the ISA version from disassembler options.
    Like my long proposed disassembler changes: adding overridable "arch"
    and "priv-spec" options, we have an option to change the ISA using the
    disassembler option.
    <https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_arch_priv_spec>


In either case, this patch set leaves both options to implement yet
supporting 'Zicntr' and 'Zihpm' extensions directly.




Along with those changes (in PATCH 2), PATCH 1 makes possible to consider
implication by using *more* information than before.  This is practically
the same as:
<https://sourceware.org/pipermail/binutils/2023-July/128715.html>,
a patch to demonstrate how to implement the 'Zce' superset extension
('Zcf' must be implied by 'Zce' ONLY IF 'F' is ALSO enabled).

The only difference is a minor type change as follows (to enable using
the "riscv_subset_supports" function).

*   Before: "const riscv_parse_subset_t *"
*   After:  "      riscv_parse_subset_t *"




Is there any other options?  Should I simplify the patch assuming we choose
the compatibility mode (Option 1) forever?

In any case, I would like to hear your thoughts.

Thanks,
Tsukasa




Tsukasa OI (2):
  RISC-V: Base for complex extension implications
  RISC-V: Add 'Zicntr' and 'Zihpm' support with compatibility measures

 bfd/elfxx-riscv.c                       |  86 +++++--
 gas/config/tc-riscv.c                   |  16 ++
 gas/testsuite/gas/riscv/march-imply-i.s |   8 +
 include/opcode/riscv-opc.h              | 310 ++++++++++++------------
 include/opcode/riscv.h                  |   1 +
 opcodes/riscv-opc.c                     |  12 +-
 6 files changed, 256 insertions(+), 177 deletions(-)


base-commit: d734d43a048b33ee12df2c06c2e782887e9715f6
-- 
2.41.0


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-21  2:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-08  3:17 [RFC PATCH 0/2] RISC-V: Add 'Zicntr' and 'Zihpm' support with compatibility measures Tsukasa OI
2023-08-08  3:17 ` [RFC PATCH 1/2] RISC-V: Base for complex extension implications Tsukasa OI
2023-08-08  3:17 ` [RFC PATCH 2/2] RISC-V: Add 'Zicntr' and 'Zihpm' support with compatibility measures Tsukasa OI
2023-10-19  7:57 ` [PING^1][RFC PATCH 0/2] " Tsukasa OI
2023-10-19  8:33   ` Nelson Chu
2023-10-20  2:52     ` Tsukasa OI
2023-10-21  0:45 ` [PATCH 0/1] " Tsukasa OI
2023-10-21  0:45   ` [PATCH 1/1] " Tsukasa OI
2023-10-21  2:17   ` [PATCH v2 0/1] " Tsukasa OI
2023-10-21  2:17     ` [PATCH v2 1/1] " Tsukasa OI

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).