This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by asb on Apr 18 2023, 1:44 AM.

Details

Summary

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

Diff Detail

Event Timeline

asb created this revision.Apr 18 2023, 1:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2023, 1:44 AM
asb requested review of this revision.Apr 18 2023, 1:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2023, 1:44 AM
asb added a reviewer: jrtc27.Apr 18 2023, 1:47 AM
joshua-arch1 accepted this revision.Apr 18 2023, 1:50 AM
This revision is now accepted and ready to land.Apr 18 2023, 1:50 AM
kito-cheng accepted this revision.Apr 18 2023, 2:24 AM
asb added a comment.Apr 18 2023, 11:04 PM

Thanks for the reviews - just to additionally note that I've checked vs GCC/binutils and it seems GCC was already sorting in this order (though required order in the -march string matching Clang's prior to D143815).

This revision was landed with ongoing or failed builds.Apr 18 2023, 11:05 PM
This revision was automatically updated to reflect the committed changes.