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

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 25165
Build 23859: 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
132

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?

133

any way of getting this automatically?

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

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

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.

@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

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

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

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