[PATCH] D148315: [RISCV] Modify arch string parsing order according to latest riscv spec

2023-04-19 Thread Jun Sha via Phabricator via cfe-commits 
joshua-arch1 added a comment.

In D148315#4279486 , @asb wrote:

> I'm starting to think we should just remove the ordering rules for z/s/x 
> altogether when parsing arch strings I see that gcc 12.2.0 actually requires 
> s and then z:
>
>   [asb@purge ~]$ riscv64-linux-gnu-gcc -march=rv64imafdc_svinval_zicbom t.c -c
>   [asb@purge ~]$ riscv64-linux-gnu-gcc -march=rv64imafdc_zicbom_svinval t.c -c
>   riscv64-linux-gnu-gcc: error: '-march=rv64imafdc_zicbom_svinval': 
> unexpected ISA string at end: 'svinval'
>
> So ISA naming strings aren't easily portable between clang and GCC right now, 
> even if the same extension names are recognised.

One of my colleagues working on GCC posted a patch to modify this order in GCC 
yesterday. Also, Binutils requires z and then s, which is inconsistent with GCC 
now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148315/new/

https://reviews.llvm.org/D148315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148315: [RISCV] Modify arch string parsing order according to latest riscv spec

2023-04-18 Thread Alex Bradbury via Phabricator via cfe-commits 
asb added a comment.

I'm starting to think we should just remove the ordering rules for z/s/x 
altogether when parsing arch strings I see that gcc 12.2.0 actually requires s 
and then z:

  [asb@purge ~]$ riscv64-linux-gnu-gcc -march=rv64imafdc_svinval_zicbom t.c -c
  [asb@purge ~]$ riscv64-linux-gnu-gcc -march=rv64imafdc_zicbom_svinval t.c -c
  riscv64-linux-gnu-gcc: error: '-march=rv64imafdc_zicbom_svinval': unexpected 
ISA string at end: 'svinval'

So ISA naming strings aren't easily portable between clang and GCC right now, 
even if the same extension names are recognised.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148315/new/

https://reviews.llvm.org/D148315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148315: [RISCV] Modify arch string parsing order according to latest riscv spec

2023-04-18 Thread Alex Bradbury via Phabricator via cfe-commits 
asb added a comment.

@joshua-arch1: I've posted D146815  to fix 
the canonical ordering and directly committed 
rGa35e67fc5be654a7efdfa6125343b90f8960a487 
 to add 
some test coverage.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148315/new/

https://reviews.llvm.org/D148315

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148315: [RISCV] Modify arch string parsing order according to latest riscv spec

2023-04-18 Thread Jun Sha via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf98ca363bed7: [RISCV] Modify arch string parsing order 
according to latest riscv spec (authored by joshua-arch1).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D148315?vs=513498&id=514555#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148315/new/

https://reviews.llvm.org/D148315

Files:
  clang/test/Driver/riscv-arch.c
  llvm/lib/Support/RISCVISAInfo.cpp


Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -758,7 +758,7 @@
   // Parse the ISA string containing non-standard user-level
   // extensions, standard supervisor-level extensions and
   // non-standard supervisor-level extensions.
-  // These extensions start with 'z', 'x', 's', 'sx' prefixes, follow a
+  // These extensions start with 'z', 's', 's', 'sx' prefixes, follow a
   // canonical order, might have a version number (major, minor)
   // and are separated by a single underscore '_'.
   // Set the hardware features for the extensions that are supported.
@@ -769,7 +769,7 @@
   OtherExts.split(Split, '_');
 
   SmallVector AllExts;
-  std::array Prefix{"z", "x", "s", "sx"};
+  std::array Prefix{"z", "s", "x", "sx"};
   auto I = Prefix.begin();
   auto E = Prefix.end();
   if (Split.size() > 1 || Split[0] != "") {
Index: clang/test/Driver/riscv-arch.c
===
--- clang/test/Driver/riscv-arch.c
+++ clang/test/Driver/riscv-arch.c
@@ -301,10 +301,10 @@
 // RV32SX-UNS: error: invalid arch name 'rv32isxabc',
 // RV32SX-UNS: unsupported non-standard supervisor-level extension 'sxabc'
 
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32ixabc_sp_sxlw -### %s \
+// RUN: %clang --target=riscv32-unknown-elf -march=rv32isp_xabc_sxlw -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32ALL %s
-// RV32ALL: error: invalid arch name 'rv32ixabc_sp_sxlw',
-// RV32ALL: unsupported non-standard user-level extension 'xabc'
+// RV32ALL: error: invalid arch name 'rv32isp_xabc_sxlw',
+// RV32ALL: unsupported standard supervisor-level extension 'sp'
 
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32i20 -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-IVER %s
@@ -351,11 +351,11 @@
 // RV32-PREFIX: error: invalid arch name 'rv32ixabc_a',
 // RV32-PREFIX: invalid extension prefix 'a'
 
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32isabc_xdef -### %s \
+// RUN: %clang --target=riscv32-unknown-elf -march=rv32ixdef_sabc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-ORDER %s
-// RV32-X-ORDER: error: invalid arch name 'rv32isabc_xdef',
-// RV32-X-ORDER: non-standard user-level extension not given
-// RV32-X-ORDER: in canonical order 'xdef'
+// RV32-X-ORDER: error: invalid arch name 'rv32ixdef_sabc',
+// RV32-X-ORDER: standard supervisor-level extension not given
+// RV32-X-ORDER: in canonical order 'sabc'
 
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32isxabc_sdef -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-S-ORDER %s
@@ -373,10 +373,10 @@
 // RV32-X-X-INVAL: error: invalid arch name 'rv32ixabc_xdef', unsupported
 // RV32-X-X-INVAL: non-standard user-level extension 'xabc'
 
-// RUN: %clang --target=riscv32-unknown-elf -march=rv32ixabc_sdef_sxghi -### 
%s \
+// RUN: %clang --target=riscv32-unknown-elf -march=rv32isdef_xabc_sxghi -### 
%s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-S-SX-INVAL %s
-// RV32-X-S-SX-INVAL: error: invalid arch name 'rv32ixabc_sdef_sxghi',
-// RV32-X-S-SX-INVAL: unsupported non-standard user-level extension 'xabc'
+// RV32-X-S-SX-INVAL: error: invalid arch name 'rv32isdef_xabc_sxghi',
+// RV32-X-S-SX-INVAL: unsupported standard supervisor-level extension 'sdef'
 
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32i -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s


Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -758,7 +758,7 @@
   // Parse the ISA string containing non-standard user-level
   // extensions, standard supervisor-level extensions and
   // non-standard supervisor-level extensions.
-  // These extensions start with 'z', 'x', 's', 'sx' prefixes, follow a
+  // These extensions start with 'z', 's', 's', 'sx' prefixes, follow a
   // canonical order, might have a version number (major, minor)
   // and are separated by a single underscore '_'.
   // Set the hardware features for the extensions that are supported.
@@ -769,7 +769,7 @@
   OtherExts.split(Split, '_');