RISC-V: Handle multi-letter extension for multilib-generator

Message ID 20200630023526.118337-1-kito.cheng@sifive.com
State New
Headers show
Series
  • RISC-V: Handle multi-letter extension for multilib-generator
Related show

Commit Message

Kito Cheng June 30, 2020, 2:35 a.m.
- The order of multi-lib config could be wrong if multi-ltter are
   used, e.g. `./multilib-generator rv32izfh-ilp32--c`, would expect
   rv32ic_zfh/ilp32 reuse rv32izfh/ilp32, however the multi-ltter is not
   handled correctly, it will generate reuse rule for rv32izfhc/ilp32
   which is invalid arch configuration.

gcc/ChangeLog:

	* config/riscv/multilib-generator (arch_canonicalize): Handle
	multi-letter extension.
	Using underline as separator between different extensions.
---
 gcc/config/riscv/multilib-generator | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

-- 
2.27.0

Comments

Jim Wilson June 30, 2020, 10:34 p.m. | #1
On Mon, Jun 29, 2020 at 7:35 PM Kito Cheng <kito.cheng@sifive.com> wrote:
>         * config/riscv/multilib-generator (arch_canonicalize): Handle

>         multi-letter extension.

>         Using underline as separator between different extensions.


This looks good to me.

> +  # Multi-letter extension must in lexicographic order.


Suggest "must in" -> "must be in"

> @@ -77,7 +91,7 @@ for cfg in sys.argv[1:]:

>    abis[abi] = 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], [])

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


Maybe add a line
  arch = arch_canonicalize (arch)
after parsing arch?

I notice with your example I get
MULTILIB_OPTIONS = march=rv32izfh/march=rv32ic_zfh mabi=ilp32
which has inconsistent handling of _ before long extensions.  This is
harmless, but this could be fixed by canonicalizing arch too.  And
that might help make the script work better with minor user input
errors.

Jim

Patch

diff --git a/gcc/config/riscv/multilib-generator b/gcc/config/riscv/multilib-generator
index b9194e6d3cc1..d4334c8847da 100755
--- a/gcc/config/riscv/multilib-generator
+++ b/gcc/config/riscv/multilib-generator
@@ -39,7 +39,6 @@  reuse = []
 canonical_order = "mafdgqlcbjtpvn"
 
 def arch_canonicalize(arch):
-  # TODO: Support Z, S, H, or X extensions.
   # TODO: Support implied extensions, e.g. D implied F in latest spec.
   # TODO: Support extension version.
   new_arch = ""
@@ -56,19 +55,34 @@  def arch_canonicalize(arch):
   long_ext_prefixes_idx = list(filter(lambda x: x != -1, long_ext_prefixes_idx))
   if long_ext_prefixes_idx:
     first_long_ext_idx = min(long_ext_prefixes_idx)
-    long_exts = arch[first_long_ext_idx:]
+    long_exts = arch[first_long_ext_idx:].split("_")
     std_exts = arch[5:first_long_ext_idx]
   else:
-    long_exts = ""
+    long_exts = []
     std_exts = arch[5:]
 
+  # Single letter extension might appear in the long_exts list,
+  # becasue we just append extensions list to the arch string.
+  std_exts += "".join(filter(lambda x:len(x) == 1, long_exts))
+
+  # Multi-letter extension must in lexicographic order.
+  long_exts = sorted(filter(lambda x:len(x) != 1, long_exts))
+
   # Put extensions in canonical order.
   for ext in canonical_order:
     if ext in std_exts:
       new_arch += ext
 
+  # Check every extension is processed.
+  for ext in std_exts:
+    if ext == '_':
+      continue
+    if ext not in canonical_order:
+      raise Exception("Unsupported extension `%s`" % ext)
+
   # Concat rest of the multi-char extensions.
-  new_arch += long_exts
+  if long_exts:
+    new_arch += "_" + "_".join(long_exts)
   return new_arch
 
 for cfg in sys.argv[1:]:
@@ -77,7 +91,7 @@  for cfg in sys.argv[1:]:
   abis[abi] = 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], [])
+  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))