From: Alex Bradbury Date: Wed, 19 Apr 2023 06:03:57 +0000 (+0100) Subject: [RISCV] Fix canonical ordering of s* vs z* extensions in RISCVISAInfo X-Git-Tag: upstream/17.0.6~11124 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c8318b973ad0414a068f4b5025a896d912554a11;p=platform%2Fupstream%2Fllvm.git [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 --- 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 --- 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")); }