Skip to content

Fixed x86-64 optimization flags for clang#13913

Merged
tgamblin merged 2 commits intospack:developfrom
alalazo:fixes/clang_flags_x86_64
Dec 4, 2019
Merged

Fixed x86-64 optimization flags for clang#13913
tgamblin merged 2 commits intospack:developfrom
alalazo:fixes/clang_flags_x86_64

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Nov 27, 2019

Before the flags used where the one for llc, the underlying compiler from LLVM IR to machine assembly. It turns out that the semantic of -march, -mtune and -mcpu changes from clang front-end to llc.

I found no definitive reference for the flags submitted in this PR, but I checked the assembly on a vectorizable function using Godbolt's web-site.

Refers to the discussion in #13825 (comment) (thanks @t-karatsu !)

Before the flags used where the one for llc, the underlying compiler
from LLVM IR to machine assembly. It turns out that the semantic of
`-march`, `-mtune` and `-mcpu` changes from clang front-end to llc.

I found no definitive reference for the flags submitted in this PR, but
I checked the assembly on a vectorizable function using Godbolt's
web-site.
@alalazo alalazo added compilers clang bugfix Something wasn't working, here's a fix labels Nov 27, 2019
@alalazo alalazo requested a review from tgamblin November 28, 2019 10:09
@hainest
Copy link
Copy Markdown
Contributor

hainest commented Dec 4, 2019

@alalazo This also causes a spurious configuration failure when building cmake because it generates a -Wunused-command-line-argument which is interpreted as a failure during compiler feature evaluation. -mcpu has also been deprecated since at least gcc-4.7 (that's the oldest one I have). Should we change it here and update the title of this PR, or make a new one?

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Dec 4, 2019

@hainest Is there any -mcpu left after the changes in this PR?

@hainest
Copy link
Copy Markdown
Contributor

hainest commented Dec 4, 2019

@hainest Is there any -mcpu left after the changes in this PR?

Not for x86, but there are for power and ARM.

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Dec 4, 2019

@hainest On those architectures as far as I know, -mcpu is not deprecated. Did you try the flags on any ppc64 or aarch64 and saw the failure?

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Dec 4, 2019

@hainest By the way, are you talking about Clang or GCC?

@hainest
Copy link
Copy Markdown
Contributor

hainest commented Dec 4, 2019

@hainest On those architectures as far as I know, -mcpu is not deprecated. Did you try the flags on any ppc64 or aarch64 and saw the failure?

I just tested with gcc-8.3 and clang-8 on ARM and gcc-7.3 and clang-9 on Power9 and confirmed that are not deprecated there. That is a rather interesting decision on their part. No changes needed, then.

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Dec 4, 2019

I still have on my TODO list another PR to fix Clang's flags for ppc64 to match GCC ones, will probably do it today. @tgamblin This is ready to go as far as I'm concerned.

@hainest
Copy link
Copy Markdown
Contributor

hainest commented Dec 4, 2019

That's probably a good idea. Keeping the usage uniform across compilers will make adding new architectures less treacherous.

@tgamblin tgamblin merged commit e9f0272 into spack:develop Dec 4, 2019
@alalazo alalazo deleted the fixes/clang_flags_x86_64 branch December 4, 2019 19:11
alalazo added a commit to alalazo/spack that referenced this pull request Dec 17, 2019
alalazo added a commit to alalazo/spack that referenced this pull request Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something wasn't working, here's a fix compilers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants