Re: [PATCH] RISC-V: Handle multi-letter extension for multilib-generator

2020-06-30 Thread Jim Wilson
On Mon, Jun 29, 2020 at 7:35 PM Kito Cheng  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] RISC-V: Handle multi-letter extension for multilib-generator

2020-06-29 Thread Kito Cheng
 - 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(-)

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