Skip to content

Disable AVX for nocona #9603

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.

Sign up for GitHub

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

Merged
esc merged 3 commits into from
Jun 10, 2024
Merged

Disable AVX for nocona #9603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f8a423
Disable AVX for nocona
sklam Jun 3, 2024
0c26b6e
Apply suggestions from code review
sklam Jun 6, 2024
50af0ab
Add test
sklam Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions numba/core/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,15 @@ def avx_default():
# on some CPUs (list at
# http://llvm.org/bugs/buglist.cgi?quicksearch=avx).
# For now we'd rather disable it, since it can pessimize code
cpu_name = ll.get_host_cpu_name()
return cpu_name not in ('corei7-avx', 'core-avx-i',
'sandybridge', 'ivybridge')
cpu_name = CPU_NAME or ll.get_host_cpu_name()

@stuartarchibald stuartarchibald Jun 4, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess that this is technically fixing a bug, in that a prescribed NUMBA_CPU_NAME was previously ignored in the logic surrounding AVX use. Should a test be added that runs if get_host_cpu_name() returns anything other than nocona and the test asserts that a prescribed NUMBA_CPU_NAME=nocona would result in config.ENABLE_AVX==False?

@sklam sklam Jun 6, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added test in 50af0ab

disabled_cpus = {'corei7-avx', 'core-avx-i',
'sandybridge', 'ivybridge'}
# Disable known baseline CPU names that virtual machines may
# incorrectly report as having AVX support.
# This can cause problems with the SVML-pass's use of AVX512.
# See https://github.com/numba/numba/issues/9582
disabled_cpus |= {'nocona'}
return cpu_name not in disabled_cpus

ENABLE_AVX = _readenv("NUMBA_ENABLE_AVX", int, avx_default)

Expand Down
28 changes: 28 additions & 0 deletions numba/tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,34 @@ def test_default_error_style_handling(self):
ex_expected = "----> new_style"
self.assertIn(ex_expected, out_msg, msg=err_msg)

@unittest.skipUnless(config.ENABLE_AVX,
"test expects NUMBA_ENABLE_AVX==True")
def test_nocona_disables_avx(self):
# test with nocona
new_env = os.environ.copy()
new_env.pop('NUMBA_ENABLE_AVX', None) # clear NUMBA_ENABLE_AVX

new_env['NUMBA_CPU_NAME'] = 'nocona'
code = ("from numba.core import config\n"
"print('---->', bool(config.ENABLE_AVX))\n"
"assert not config.ENABLE_AVX")
out, err = run_in_subprocess(dedent(code), env=new_env)

@gmarkall gmarkall Jun 10, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not suggesting changing anything now, especially as this is already RTM, but I think @TestCase.run_in_subprocess is a simpler way of doing this - it allows env vars for the subprocess to be set in the decorator args.

err_msg = err.decode('utf-8')
out_msg = out.decode('utf-8')
ex_expected = "----> False"
self.assertIn(ex_expected, out_msg, msg=err_msg)

# test with skylake-avx512
new_env['NUMBA_CPU_NAME'] = 'skylake-avx512'
code = ("from numba.core import config\n"
"print('---->', bool(config.ENABLE_AVX))\n"
"assert config.ENABLE_AVX")
out, err = run_in_subprocess(dedent(code), env=new_env)
err_msg = err.decode('utf-8')
out_msg = out.decode('utf-8')
ex_expected = "----> True"
self.assertIn(ex_expected, out_msg, msg=err_msg)


class TestNumbaOptLevel(TestCase):
# Tests that the setting of NUMBA_OPT influences the "cheap" module pass.
Expand Down