[RISCV] Fix canonical ordering of s* vs z* extensions in RISCVISAInfo

As noted in https://reviews.llvm.org/D148315, the ordering logic for
OrderedExtensionMap currently puts s* before z* extensions, but per the
ISA manual the correct order should be z* and then s* (with the
exception of zxm*, which are ordered after s*).

This patch fixes the ordering and adds a TODO for zxm*. The changes are
visible in the test case added in
a35e67fc5be654a7efdfa6125343b90f8960a487 which also demonstrates an
issue with the ordering of single letter extensions (which isn't
addressed in this patch).

This ordering matches the one used by GCC/binutils as well.

Differential Revision: https://reviews.llvm.org/D148615

diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index f3f53db..d5523ef 100644 (file)
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -286,16 +286,17 @@ bool RISCVISAInfo::hasExtension(StringRef Ext) const {
 // We rank extensions in the following order:
 // -Single letter extensions in canonical order.
 // -Unknown single letter extensions in alphabetical order.
-// -Multi-letter extensions starting with 's' in alphabetical order.
 // -Multi-letter extensions starting with 'z' sorted by canonical order of
 //  the second letter then sorted alphabetically.
+// -Multi-letter extensions starting with 's' in alphabetical order.
+// -(TODO) Multi-letter extensions starting with 'zxm' in alphabetical order.
 // -X extensions in alphabetical order.
 // These flags are used to indicate the category. The first 6 bits store the
 // single letter extension rank for single letter and multi-letter extensions
 // starting with 'z'.
 enum RankFlags {
-  RF_S_EXTENSION = 1 << 6,
-  RF_Z_EXTENSION = 1 << 7,
+  RF_Z_EXTENSION = 1 << 6,
+  RF_S_EXTENSION = 1 << 7,
   RF_X_EXTENSION = 1 << 8,
 };
 

diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index afdf308..44f1abd 100644 (file)
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -484,11 +484,8 @@ TEST(OrderedExtensionMap, ExtensionsAreCorrectlyOrdered) {
   for (const auto &Ext : Exts)
     ExtNames.push_back(Ext.first);
 
-  // FIXME: z* extensions should be ordered before s* extensions. The current
-  // ordering matches what is documented in RISCVISAInfo, but this doesn't
-  // match the ISA manual.
   // FIXME: 'l' and 'y' should be ordered after 'i', 'm', 'c'.
   EXPECT_THAT(ExtNames,
-              ElementsAre("i", "m", "l", "c", "y", "sbar", "sfoo", "zicsr",
-                          "zmfoo", "zfinx", "zzfoo", "xbar", "xfoo"));
+              ElementsAre("i", "m", "l", "c", "y", "zicsr", "zmfoo", "zfinx",
+                           "zzfoo", "sbar", "sfoo", "xbar", "xfoo"));
 }