Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cctools: try to use clang in as #3486

Merged
merged 1 commit into from
Jan 28, 2019

Conversation

jmroot
Copy link
Member

@jmroot jmroot commented Jan 24, 2019

The clang integrated assembler is way more up-to-date than the ancient
pre-GPLv3 version of gas that is the alternative. The latter doesn't
know about newer instructions such as AVX. The as(1) shipped with
recent Xcode versions will just run clang behind the scenes under most
circumstances for this reason.

Our cctools can't actually depend on clang because that would create a
circular dependency. So we'll try to run the clang corresponding to the
LLVM being used by cctools, and if that isn't present, try the system
clang in /usr/bin if we're on an OS version that has a clang with a
working integrated assembler. (The ancient gas is still used as a final
fallback.)

Fixes: https://trac.macports.org/ticket/37846

Type(s)
  • enhancement
Tested on

macOS 10.14
Xcode 10.1

The clang integrated assembler is way more up-to-date than the ancient
pre-GPLv3 version of gas that is the alternative. The latter doesn't
know about newer instructions such as AVX. The as(1) shipped with
recent Xcode versions will just run clang behind the scenes under most
circumstances for this reason.

Our cctools can't actually depend on clang because that would create a
circular dependency. So we'll try to run the clang corresponding to the
LLVM being used by cctools, and if that isn't present, try the system
clang in /usr/bin if we're on an OS version that has a clang with a
working integrated assembler. (The ancient gas is still used as a final
fallback.)

Fixes: https://trac.macports.org/ticket/37846
@jmroot jmroot requested a review from jeremyhu January 24, 2019 15:16
@macportsbot
Copy link

Notifying maintainers:
@jeremyhu for port cctools.

@macportsbot macportsbot added by: member Created by a member with commit rights maintainer: open Affects an openmaintainer port type: enhancement labels Jan 24, 2019
@macportsbot
Copy link

Travis Build #5004 Passed.

Lint results
--->  Verifying Portfile for cctools
--->  0 errors and 0 warnings found.

Port cctools success on xcode9.4. Log
Port cctools success on xcode8.3. Log
Port cctools success on xcode7.3. Log
Port cctools success on xcode10.1. Log

@cjones051073
Copy link
Member

LGTM... I like the way you try and find the correct MPs clang version, without directly depending on it, and fall back to trying /usr/bin/clang instead if not found.

With this patch I was successfully able to build OpenBLAS with AVX2 enabled, which currently has to be turned off due to the fact the symbols generated by gcc's fortran compiler could not be linked, due to the limitations of cctools.

https://trac.macports.org/ticket/57912

I think we should not sit on this for too long, and merge quickly.

@kencu What do you think ?

@jmroot
Copy link
Member Author

jmroot commented Jan 27, 2019

I'll merge if @jeremyhu doesn't raise any objections by the end of the day.

@kencu
Copy link
Contributor

kencu commented Jan 27, 2019

a workable approach. The only other approach I could think of would be to have "as" respond to an env var with a clang path in it, and that seems more cumbersome. So +1 .

@cjones051073
Copy link
Member

Its unlikely we will hear from @jeremyhu, if previous experience holds. I am happy with the changes here, so if no one else beats me to it, or objects, I am happy to be the one to hit go later on today...

@cjones051073 cjones051073 merged commit 7aa2ff6 into macports:master Jan 28, 2019
@jmroot jmroot deleted the cctools-try-clang branch January 28, 2019 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
by: member Created by a member with commit rights maintainer: open Affects an openmaintainer port type: enhancement
Development

Successfully merging this pull request may close these issues.

5 participants