Sync target triple generation with the version in Makefile.inc1.
ClosedPublic
Actions

Authored by jhb on Nov 12 2019, 5:10 PM.

Details

Reviewers

dim

Commits

rS354662: Sync target triple generation with the version in Makefile.inc1.

Test Plan

Noticed a build of riscv64sf used a default triple of 'riscv64sf-*' which wouldn't have worked. Now uses a proper triple.
Tested that an aarch64 cross-build still worked, so the aarch64 expression that was removed is not needed.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhb created this revision.Nov 12 2019, 5:10 PM

Harbormaster completed remote builds in B27476: Diff 64242.Nov 12 2019, 5:10 PM

I'm fine with this, though the substitution expression is getting more and more unreadable... I'm having difficulty to figure out what the actual triple is going to end up as; is it risc64-x-y ?

At some point we might want to rewrite this part with regular .if and .else clauses, I guess. :)

This revision is now accepted and ready to land.Nov 12 2019, 5:33 PM

It would be nice if there was some way to centralize this modifier, maybe by using an approach like __LLVM_TARGET_FILT since it is used twice in Makefile.inc1 and twice here. The POWER9 folks have some other review open that centralizes target triple generation I thought, but I haven't looked at it recently.

Oh, and yes, for riscv it always ends up as 'riscv64-unknown-freebsd13.0' for both riscv64 and riscv64sf. We add -msoft-float to CFLAGS in bsd.cpu.mk for riscv64sf. I'm not sure if there's a way to encode the float ABI in the target triple. (For 32-bit arm at least there are ABI variants you can put into the triple)

Closed by commit rS354662: Sync target triple generation with the version in Makefile.inc1. (authored by jhb). · Explain WhyNov 12 2019, 9:35 PM

This revision was automatically updated to reflect the committed changes.

jhb added a commit: rS354662: Sync target triple generation with the version in Makefile.inc1..

Herald added a subscriber: imp. · View Herald TranscriptNov 12 2019, 9:35 PM

dim mentioned this in rS356460: Merge llvm, clang, compiler-rt, libc++, libunwind, lld, lldb and openmp.Jan 7 2020, 6:13 PM

dim mentioned this in rS360658: Merge llvm, clang, compiler-rt, libc++, libunwind, lld, lldb and openmp.May 5 2020, 1:05 PM