RISC-V: Handle extensions combination correctly in multilib-generator.
diff mbox series

Message ID 20190805082017.26702-1-kito.cheng@sifive.com
State New
Headers show
Series
  • RISC-V: Handle extensions combination correctly in multilib-generator.
Related show

Commit Message

Kito Cheng Aug. 5, 2019, 8:20 a.m. UTC
Input string consist of four parts:
  <primary arch>-<abi>-<additional arches>-<extensions>

and generator doing combination with <extensions> part, but it just
append the extension at the end of arch string, it might generate
invalid arch.

For example, without this patch `./multilib-generator rv32imafc-ilp32--d`
will generate:

MULTILIB_OPTIONS = march=rv32imafc/march=rv32imafcd mabi=ilp32
                                         ^^^^^^^^^^

and rv32imafcd is not in canonical order.

Tested with python 2.7 and python 3.6/3.7.

gcc/ChangeLog

	* gcc/config/riscv/multilib-generator: Handle extensions
	combination correctly.
---
 gcc/config/riscv/multilib-generator | 37 +++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Andreas Schwab Aug. 5, 2019, 8:55 a.m. UTC | #1
On Aug 05 2019, Kito Cheng <kito.cheng@sifive.com> wrote:

> +  # TODO: Support implied extensions, e.g. D implied F in latest spce.

spec

> +  # TODO: Support extesnion version.

extension

> +  # Filter out any non exist index.

non-existent

Andreas.
Jim Wilson Aug. 5, 2019, 9:48 p.m. UTC | #2
On Mon, Aug 5, 2019 at 1:20 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> gcc/ChangeLog
>         * gcc/config/riscv/multilib-generator: Handle extensions
>         combination correctly.

A ChangeLog entry should generally describe what a patch changes, not
what it does.  So this should mention a new variable canonical_order,
a new function arch_canonicalize, and the call to pass alts through
it.

> +    raise Exception("Unexpect arch: `%d`" % arch[:5])

Unexpect -> Unexpected

> +  long_ext_prefixs = ['z', 's', 'h', 'x']

prefixs -> prefixes

> +  # Concat rest multi-char extensions.

rest multi-char -> rest of the multi-char

This looks good to me with the minor English improvements Andreas and
I suggested.

Jim
Kito Cheng Aug. 6, 2019, 3:17 a.m. UTC | #3
Hi Jim. Andreas:

Thanks your review :)

Committed with English improvements and ChangeLog update as r274137

On Tue, Aug 6, 2019 at 5:48 AM Jim Wilson <jimw@sifive.com> wrote:
>
> On Mon, Aug 5, 2019 at 1:20 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> > gcc/ChangeLog
> >         * gcc/config/riscv/multilib-generator: Handle extensions
> >         combination correctly.
>
> A ChangeLog entry should generally describe what a patch changes, not
> what it does.  So this should mention a new variable canonical_order,
> a new function arch_canonicalize, and the call to pass alts through
> it.
>
> > +    raise Exception("Unexpect arch: `%d`" % arch[:5])
>
> Unexpect -> Unexpected
>
> > +  long_ext_prefixs = ['z', 's', 'h', 'x']
>
> prefixs -> prefixes
>
> > +  # Concat rest multi-char extensions.
>
> rest multi-char -> rest of the multi-char
>
> This looks good to me with the minor English improvements Andreas and
> I suggested.
>
> Jim

Patch
diff mbox series

diff --git a/gcc/config/riscv/multilib-generator b/gcc/config/riscv/multilib-generator
index a711153b3a3..52cef276509 100755
--- a/gcc/config/riscv/multilib-generator
+++ b/gcc/config/riscv/multilib-generator
@@ -36,6 +36,41 @@  abis = collections.OrderedDict()
 required = []
 reuse = []
 
+canonical_order = "mafdqlcbjtpvn"
+
+def arch_canonicalize(arch):
+  # TODO: Support Z, S, H, or X extensions.
+  # TODO: Support implied extensions, e.g. D implied F in latest spce.
+  # TODO: Support extesnion version.
+  new_arch = ""
+  if arch[:5] in ['rv32e', 'rv32i', 'rv64i']:
+    new_arch = arch[:5]
+  else:
+    raise Exception("Unexpect arch: `%d`" % arch[:5])
+
+  # Find any Z, S, H or X
+  long_ext_prefixs = ['z', 's', 'h', 'x']
+  long_ext_prefixs_idx = map(lambda x: arch.find(x), long_ext_prefixs)
+
+  # Filter out any non exist index.
+  long_ext_prefixs_idx = list(filter(lambda x: x != -1, long_ext_prefixs_idx))
+  if long_ext_prefixs_idx:
+    first_long_ext_idx = min(long_ext_prefixs_idx)
+    long_exts = arch[first_long_ext_idx:]
+    std_exts = arch[5:first_long_ext_idx]
+  else:
+    long_exts = ""
+    std_exts = arch[5:]
+
+  # Put extensions in canonical order.
+  for ext in canonical_order:
+    if ext in std_exts:
+      new_arch += ext
+
+  # Concat rest multi-char extensions.
+  new_arch += long_exts
+  return new_arch
+
 for cfg in sys.argv[1:]:
   (arch, abi, extra, ext) = cfg.split('-')
   arches[arch] = 1
@@ -43,7 +78,9 @@  for cfg in sys.argv[1:]:
   extra = list(filter(None, extra.split(',')))
   ext = list(filter(None, ext.split(',')))
   alts = sum([[x] + [x + y for y in ext] for x in [arch] + extra], [])
+  # TODO: We should expand g to imadzifencei once we support newer spec.
   alts = alts + [x.replace('imafd', 'g') for x in alts if 'imafd' in x]
+  alts = list(map(arch_canonicalize, alts))
   for alt in alts[1:]:
     arches[alt] = 1
     reuse.append('march.%s/mabi.%s=march.%s/mabi.%s' % (arch, abi, alt, abi))