This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Change how mtune aliases are implemented.
ClosedPublic

Authored by craig.topper on Aug 11 2022, 11:50 AM.

Details

Summary

The previous implementation translated from names like sifive-7-series
to sifive-7-rv32 or sifive-7-rv64. This also required sifive-7-rv32
and sifive-7-rv64 to be valid CPU names. As those are not real
CPUs it doesn't make sense to accept them in -mcpu.

This patch does away with the translation and adds sifive-7-series
directly to RISCV.td. Removing sifive-7-rv32 and sifive-7-rv64.
sifive-7-series is only allowed in -mtune.

I've also added "rocket" to RISCV.td but have not removed rocket-rv32
or rocket-rv64.

To prevent -mcpu=sifive-7-series or -mcpu=rocket being used with llc,
I've added a Feature32Bit to all rv32 CPUs. And made it an error to
have an rv32 triple without Feature32Bit. sifive-7-series and rocket
do not have Feature32Bit or Feature64Bit set so the user would need
to provide -mattr=+32bit or -mattr=+64bit along with the -mcpu to
avoid the error.

SiFive no longer names their newer products with 3, 5, or 7 series.
Instead we have p200 series, x200 series, p500 series, and p600 series.
Following the previous behavior would require a sifive-p500-rv32 and
sifive-p500-rv64 in order to support -mtune=sifive-p500-series. There
is currently no p500 product, but it could start getting confusing if
there was in the future.

I'm open to hearing alternatives for how to achieve my main goal
of removing sifive-7-rv32/rv64 as a CPU name.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 11 2022, 11:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 11 2022, 11:50 AM
craig.topper requested review of this revision.Aug 11 2022, 11:50 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 11 2022, 11:50 AM
kito-cheng added inline comments.Aug 15 2022, 12:24 AM
clang/test/Driver/riscv-cpus.c
27

I got error: unknown target CPU 'rocket' (e.g. clang --target=riscv64 -mtune=rocket ~/hello.c) after this patch, but no error if come with -###

craig.topper planned changes to this revision.Aug 15 2022, 9:45 PM
craig.topper added inline comments.
clang/test/Driver/riscv-cpus.c
27

Thanks. I failed to update checkTuneCPUKind so the CPU doesn't validate.

Handle TUNE_PROC in checkTuneCPUKind

asb added a comment.Aug 17 2022, 5:52 AM

This seems like a sensible direction to me - let's discuss in the sync-up call this Thursday to check everyone is on-board and doesn't have concerns.

Add release note about sifive-7-rv32/64 being removed

craig.topper edited the summary of this revision. (Show Details)Aug 18 2022, 11:50 AM
reames accepted this revision.Aug 18 2022, 1:17 PM
reames added a subscriber: reames.

LGTM

clang/docs/ReleaseNotes.rst
197 ↗(On Diff #453746)

Can you given any guidance on what to use instead?

This revision is now accepted and ready to land.Aug 18 2022, 1:17 PM
This revision was landed with ongoing or failed builds.Aug 18 2022, 4:28 PM
This revision was automatically updated to reflect the committed changes.