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

arch: Correct march to mcpu for ppc #2174

Merged
merged 2 commits into from
Aug 8, 2023
Merged

Conversation

raminammour
Copy link
Contributor

This should be a very minor change: Switch between -march=native (x86) and -mcpu=native -mtune=native (PowerPC)

fixes #2146

I am not sure if you are testing on PPC (I guess not since the error never showed up) so not sure what to do for an appropriate test.

@mloubout mloubout added the arch jitting, archinfo, ... label Jul 27, 2023
Copy link
Contributor

@mloubout mloubout left a comment

Choose a reason for hiding this comment

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

@@ -396,6 +396,12 @@ def __init__(self, *args, **kwargs):
# from `=512`
self.cflags.append('-mprefer-vector-width=512')

if platform in [POWER8, POWER9]:
# -march isn't supported on power architectures
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation looks off, will break CI

@raminammour raminammour changed the title Correct march to mcpu for ppc arch: Correct march to mcpu for ppc Jul 27, 2023
@FabioLuporini
Copy link
Contributor

Thanks -- content is uncontroversial!

As @mloubout suggested, could you rebase to honor our commit message format? Thanks a lot

@@ -382,7 +382,7 @@ def __init__(self, *args, **kwargs):

platform = kwargs.pop('platform', configuration['platform'])

self.cflags += ['-march=native', '-Wno-unused-result',
self.cflags += ['-Wno-unused-result',
Copy link
Contributor

Choose a reason for hiding this comment

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

This may also break flake8?

@codecov
Copy link

codecov bot commented Jul 31, 2023

Codecov Report

Merging #2174 (7caa50c) into master (a1506b6) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #2174      +/-   ##
==========================================
- Coverage   87.09%   87.08%   -0.01%     
==========================================
  Files         226      226              
  Lines       40172    40175       +3     
  Branches     7330     7331       +1     
==========================================
+ Hits        34987    34988       +1     
- Misses       4605     4606       +1     
- Partials      580      581       +1     
Files Changed Coverage Δ
devito/arch/compiler.py 38.91% <50.00%> (-0.04%) ⬇️

@mloubout
Copy link
Contributor

mloubout commented Aug 4, 2023

Rebase onto master with a single commit (the last one is good) and GTG

Switch between -march=native (x86) and -mcpu=native -mtune=native (PowerPC)

Costmetic changes so `mcpu` and `march` remain the first flag

Fix indentation

arch: Correct march to mcpu for ppc

Correcting commit message since editing the title on github webui wasn't enough (I couldn't figure out how to rebase from the webui, sorry!).

Maybe you can squash to this commit when merging?
@mloubout mloubout merged commit fde335d into devitocodes:master Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch jitting, archinfo, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error on ppc from gcc flag march=native
4 participants