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

[Alex Bradbury via Phabricator via llvm-commits]

asb created this revision.
asb added reviewers: craig.topper, reames, kito-cheng, joshua-arch1.
Herald added subscribers: jobnoorman, luke, wingo, pmatos, VincentWu, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

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 <https://reviews.llvm.org/rGa35e67fc5be654a7efdfa6125343b90f8960a487> which also demonstrates an issue with the ordering of single letter extensions (which isn't addressed in this patch).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148615

Files:
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/unittests/Support/RISCVISAInfoTest.cpp


Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===================================================================
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -484,11 +484,8 @@
   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"));
 }
Index: llvm/lib/Support/RISCVISAInfo.cpp
===================================================================
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -286,16 +286,17 @@
 // 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,
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148615.514561.patch
Type: text/x-patch
Size: 1976 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230418/7883614e/attachment.bin>