Skip to content

cpu: fix clang flags for generic x86_64#13491

Merged
becker33 merged 2 commits intodevelopfrom
bugfix/clang-generic-x86-flags
Oct 30, 2019
Merged

cpu: fix clang flags for generic x86_64#13491
becker33 merged 2 commits intodevelopfrom
bugfix/clang-generic-x86-flags

Conversation

@becker33
Copy link
Copy Markdown
Member

@becker33 becker33 commented Oct 29, 2019

The x86_64 target for clang currently provides architecture flags -march=x86_64 -mcpu=generic. The -mcpu=generic flag is ignored by clang (and seems to be a mistaken copy/paste from GCC.

This PR removes that flag.

@becker33 becker33 requested a review from alalazo October 29, 2019 19:52
@alalazo
Copy link
Copy Markdown
Member

alalazo commented Oct 29, 2019

It's available according to llc:

$ llc-3.9 -march=x86-64 -mcpu=help
Available CPUs for this target:

  amdfam10       - Select the amdfam10 processor.
  athlon         - Select the athlon processor.
  athlon-4       - Select the athlon-4 processor.
  athlon-fx      - Select the athlon-fx processor.
  athlon-mp      - Select the athlon-mp processor.
  athlon-tbird   - Select the athlon-tbird processor.
  athlon-xp      - Select the athlon-xp processor.
  athlon64       - Select the athlon64 processor.
  athlon64-sse3  - Select the athlon64-sse3 processor.
  atom           - Select the atom processor.
  barcelona      - Select the barcelona processor.
  bdver1         - Select the bdver1 processor.
  bdver2         - Select the bdver2 processor.
  bdver3         - Select the bdver3 processor.
  bdver4         - Select the bdver4 processor.
  bonnell        - Select the bonnell processor.
  broadwell      - Select the broadwell processor.
  btver1         - Select the btver1 processor.
  btver2         - Select the btver2 processor.
  c3             - Select the c3 processor.
  c3-2           - Select the c3-2 processor.
  cannonlake     - Select the cannonlake processor.
  core-avx-i     - Select the core-avx-i processor.
  core-avx2      - Select the core-avx2 processor.
  core2          - Select the core2 processor.
  corei7         - Select the corei7 processor.
  corei7-avx     - Select the corei7-avx processor.
  generic        - Select the generic processor.
  geode          - Select the geode processor.
  haswell        - Select the haswell processor.
[ ... ]

@becker33
Copy link
Copy Markdown
Member Author

becker33 commented Oct 29, 2019

llc is not available on MacOS, so it's hard to tell why it's not recognized. It looks like it doesn't matter what the argument to -mcpu is, that flag is not accepted on MacOS

@scheibelp
Copy link
Copy Markdown
Member

scheibelp commented Oct 30, 2019

This appears to be failing for one of the test_optimization_flags parameterizations

alalazo
alalazo reviewed Oct 30, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a discussion with @becker33 this is due to differences between Apple's Clang and upstream Clang, in particular on the underlying linker and assembler. Suggesting below a different way to deal with it.

@becker33
Copy link
Copy Markdown
Member Author

becker33 commented Oct 30, 2019

For reference, this issue is also causing build failures for the diffutils package. So if anyone sees strange diffutils failures in configure, this could be the cause.

becker33
becker33 commented Oct 30, 2019
View reviewed changes
Copy link
Copy Markdown
Member Author

@becker33 becker33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new version looks good to me, I can't approve it though since I'm the PR author and github doesn't like that.

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Oct 30, 2019

@becker33 I'd say let's merge this if diffutils or other packages that now fail are building fine for you.

@becker33
Copy link
Copy Markdown
Member Author

becker33 commented Oct 30, 2019

Ok.

@tgamblin this should probably be backported to v0.13 for the next patch release. Have we settled on a way of marking such things?

@becker33 becker33 merged commit 230c6aa into develop Oct 30, 2019
tgamblin pushed a commit that referenced this pull request Nov 1, 2019
@becker33 @tgamblin
cpu: fix clang flags for generic x86_64 (#13491)
b727f92 
* cpu: differentiate flags used for pristine LLVM vs. Apple's version
@tgamblin tgamblin deleted the bugfix/clang-generic-x86-flags branch December 24, 2019 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalazo alalazo alalazo left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@becker33 @alalazo @scheibelp @adamjstewart