Page MenuHomeFreeBSD

Fix TARGET_TRIPLE variable assembly when specifying TARGET_ABI
Needs ReviewPublic

Authored by alfredo.junior_eldorado.org.br on Feb 18 2019, 7:30 PM.

Details

Summary

Variable TARGET_TRIPLE is used as argument to clang's "-target" parameter specially when cross compiling.

It's expected to be in the form "<arch><sub>-<vendor>-<sys>-<abi>" (see https://clang.llvm.org/docs/CrossCompilation.html#target-triple), but it's currently calculated as "<arch><sub>-<abi>-<sys>"

*note <sub> is ignored in this patch as well

Test Plan

buildworld powerpc64-elfv2 (cross)
buildworld x86_64 (native)

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 22558
Build 21692: arc lint + arc unit

Event Timeline

alfredo.junior_eldorado.org.br retitled this revision from Fix TARGET_TRIPLE variable computation when specifying TARGET_ABI to Fix TARGET_TRIPLE variable assembly when specifying TARGET_ABI.Feb 18 2019, 7:41 PM
alfredo.junior_eldorado.org.br edited the summary of this revision. (Show Details)
alfredo.junior_eldorado.org.br edited the test plan for this revision. (Show Details)
bdragon accepted this revision.Feb 18 2019, 8:57 PM

Yeah, that's better.

Since I'm not an official member, make sure and get more approvals.

This will need verification on mips and arm I believe, to ensure nothing breaks.

This revision is now accepted and ready to land.Feb 18 2019, 8:57 PM
emaste added subscribers: jhb, markj.Feb 27 2019, 5:15 PM
imp added inline comments.Feb 27 2019, 5:32 PM
Makefile.inc1
133

any way of getting this automatically?

148

not an objection, per-se, but an observation that these same transforms are used 4 times in an identical fasion. Any way to make that more common?

jhb added inline comments.Feb 27 2019, 6:44 PM
Makefile.inc1
133

Not really, it's the version we are building as opposed to the build host, etc. We already have to bump this one when creating a new branch, this just centralizes it. I think Makefile.libcompat also hardcodes freebsd13 and it would be nice if we could fix that to use OS_VERSION as part of this change.

148

You could have TARGET_TRIPLE_ARCH and MACHINE_TRIPLE_ARCH helper variables to at least cut it in half.

@alfredo.junior_eldorado.org.br think you can make the changes @jhb and @imp are suggesting? Looks fine to me overall.

I updated the patch to retrieve FreeBSD version automatically, however I'm not sure on how to better address 'lib/clang/llvm.build.mk'. The variables created on 'Makefile.inc1' are not visible there.

This revision now requires review to proceed.Jul 1 2019, 8:29 PM
alfredo.junior_eldorado.org.br marked 4 inline comments as done.Jul 1 2019, 8:29 PM

TARGET=powerpc TARGET_ARCH=powerpc64 produces:

clang -target powerpc64-unknown-freebsd13.0

TARGET=powerpc TARGET_ARCH=powerpc64 TARGET_ABI=elfv2 produces:

clang -target powerpc64-unknown-freebsd13.0-elfv2

remove a level of identation

bdragon added inline comments.Sat, Sep 7, 10:39 PM
share/mk/sys.mk
352 ↗(On Diff #59315)

This grep doesn't seem to actually work for some reason. It isn't picking up SRCTOP...

bdragon added inline comments.Tue, Sep 10, 1:44 PM
share/mk/sys.mk
352 ↗(On Diff #59315)

of course now I'm having trouble reproducing this. I distinctly remember seeing lots of "grep: /sys/sys/param.h not found" but I'm not seeing it anymore.

imp added a comment.Mon, Sep 16, 7:15 PM

So I think this belongs in share/mk/src.sys.env.mk, but I'm not sure.

share/mk/sys.mk
352 ↗(On Diff #59315)

One $ is what you want since SRCTOP is a makefile variable.
However, I'm 100% sure this is the wrong place for this.
Also, just use newvers.sh to get this info.
And I think you want is something like

.if !defined(OS_VERSION)
OS_VERSION!=${SRCTOP}/sys/conf/newvers.sh -V REVISION
.export OS_VERSION
.endif