This is an archive of the discontinued LLVM Phabricator instance.

Add support for openSUSE RISC-V triple
ClosedPublic

Authored by schwab on Jun 18 2019, 9:04 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

schwab created this revision.Jun 18 2019, 9:04 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 18 2019, 9:04 AM
lebedev.ri added a subscriber: lebedev.ri.

Nice :)
It is also a good idea to upload patches with full context (-U99999)

llvm/unittests/ADT/TripleTest.cpp
333 ↗(On Diff #205315)

Without context i can't tell, in general is the grouping here based on the triple, or the vendor?

asb added a comment.Jun 18 2019, 9:03 PM

Thanks for the contribution! I'd recommend adding a skeleton toolchain dir structure to clang/test/Driver/Inputs and adding a test to clang/test/Driver/riscv64-toolchain.c. You should be able to see examples in test/Driver/Inputs.

schwab updated this revision to Diff 206173.Jun 24 2019, 2:40 AM
schwab updated this revision to Diff 206423.Jun 25 2019, 5:39 AM

Test added

asb accepted this revision.Jul 7 2019, 9:30 PM

LGTM, thanks!

This revision is now accepted and ready to land.Jul 7 2019, 9:30 PM
This revision was automatically updated to reflect the committed changes.
lenary added a comment.Aug 1 2019, 7:25 AM

Thanks @schwab, sorry we took so long to merge your patch, but it's merged now!

MaskRay added a subscriber: MaskRay.EditedSep 10 2021, 12:50 AM

hexchain pointed me to this patch and asked why Suse can add a triple here... So here are some explanations:

We should avoid adding more target triples to CollectLibDirsAndTriples.
Every riscv64 user will waste some fstat or openat.
(If there are 100 distros customizing riscv64, apparently we shouldn't enumerate them.)

The list is mainly there so that clang --target=riscv64 can magically pick riscv64-linux-gnu (and other RISCV64Triples triples).
This behavior is really discouraged for newer architectures and OSes.

They should just specify the full triple clang --target=riscv64-suse-linux. This doesn't require you to add anything to the list.

If on riscv64-suse-linux, clang for some reason doesn't recognize riscv64-suse-linux.
The correct fix is to make LLVM_DEFAULT_TARGET_TRIPLE riscv64-suse-linux.
This doesn't need any hardcoded value from RISCV64Triples.


So why cannot we clean these RISCV64Triples?
Perhaps the reason is that some users are unfortunately relying on the behavior.
Deleting these values will be the correct way forward but they can be unhappy temporarily.

rkruppe removed a subscriber: rkruppe.Sep 10 2021, 7:25 AM

I created D109727 to clean up the *Triples variables.