Skip to content

[openmp] __kmp_x86_cpuid fix for i386/PIC builds. #84626

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

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

devnexen
Copy link
Member

@devnexen devnexen commented Mar 9, 2024

No description provided.

@llvmbot llvmbot added the openmp:libomp OpenMP host runtime label Mar 9, 2024
@devnexen devnexen force-pushed the openmp_cpuid_386_fix branch from 17b22a3 to a833508 Compare March 9, 2024 12:22
Copy link

github-actions bot commented Mar 9, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@devnexen devnexen force-pushed the openmp_cpuid_386_fix branch 2 times, most recently from 1fa2a7c to f6d36ac Compare March 9, 2024 12:31
@devnexen devnexen force-pushed the openmp_cpuid_386_fix branch from f6d36ac to f763725 Compare March 9, 2024 12:34
@devnexen devnexen requested a review from shiltian March 10, 2024 09:57
@brad0
Copy link
Contributor

brad0 commented Mar 11, 2024

cc @MaskRay

@devnexen devnexen merged commit facb89a into llvm:main Mar 11, 2024
devnexen added a commit to devnexen/llvm-project that referenced this pull request Mar 13, 2024
@mgorny
Copy link
Member

mgorny commented Mar 16, 2024

I'm sorry to say but this change actually broke 32-bit builds on Gentoo, when using GCC. Most of the tests segfault now, e.g.:

0/1] cd /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test && /tmp/portage/sys-libs/libomp-19.0.0.9999/temp/python3.11/bin/python3 /usr/bin/lit -vv -j 12 /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test
-- Testing: 325 tests, 12 workers --
FAIL: libomp :: affinity/format/api.c (1 of 325)
******************** TEST 'libomp :: affinity/format/api.c' FAILED ********************
Exit Code: -11

Command Output (stdout):
--
# RUN: at line 1
/usr/lib/ccache/bin/i686-pc-linux-gnu-clang -fopenmp   -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/src -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test -L /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/src  -fno-omit-frame-pointer -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test/ompt /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test/affinity/format/api.c -o /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test/affinity/format/Output/api.c.tmp -lm -latomic && /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test/affinity/format/Output/api.c.tmp
# executed command: /usr/lib/ccache/bin/i686-pc-linux-gnu-clang -fopenmp -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/src -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test -L /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/src -fno-omit-frame-pointer -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test/ompt /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test/affinity/format/api.c -o /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test/affinity/format/Output/api.c.tmp -lm -latomic
# executed command: /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test/affinity/format/Output/api.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11

--

********************

My educated guess is that you aren't listing edi in clobbers. If I do that, most of the tests start passing again but I still get a handful of segfaults.

@devnexen
Copy link
Member Author

I'm sorry to say but this change actually broke 32-bit builds on Gentoo, when using GCC. Most of the tests segfault now, e.g.:

0/1] cd /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test && /tmp/portage/sys-libs/libomp-19.0.0.9999/temp/python3.11/bin/python3 /usr/bin/lit -vv -j 12 /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test
-- Testing: 325 tests, 12 workers --
FAIL: libomp :: affinity/format/api.c (1 of 325)
******************** TEST 'libomp :: affinity/format/api.c' FAILED ********************
Exit Code: -11

Command Output (stdout):
--
# RUN: at line 1
/usr/lib/ccache/bin/i686-pc-linux-gnu-clang -fopenmp   -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/src -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test -L /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/src  -fno-omit-frame-pointer -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test/ompt /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test/affinity/format/api.c -o /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test/affinity/format/Output/api.c.tmp -lm -latomic && /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test/affinity/format/Output/api.c.tmp
# executed command: /usr/lib/ccache/bin/i686-pc-linux-gnu-clang -fopenmp -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/src -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test -L /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/src -fno-omit-frame-pointer -I /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test/ompt /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp/runtime/test/affinity/format/api.c -o /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test/affinity/format/Output/api.c.tmp -lm -latomic
# executed command: /tmp/portage/sys-libs/libomp-19.0.0.9999/work/openmp_build-abi_x86_32.x86/runtime/test/affinity/format/Output/api.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11

--

********************

My educated guess is that you aren't listing edi in clobbers. If I do that, most of the tests start passing again but I still get a handful of segfaults.

What happens if you do the following instead ?

__asm__ __volatile__("pushl  %%ebx\n"
		"cpuid\n"
		"mov    %%ebx,%1\n"
		"popl   %%ebx"
                 : "=a"(p->eax), "=b"(p->ebx), "=c"(p->ecx), "=d"(p->edx)
                 : "a"(leaf), "c"(subleaf));

@mgorny
Copy link
Member

mgorny commented Mar 16, 2024

Actually, this change makes no sense to me (either the committed or the proposed variant). You're explicitly specifying ebx as the output operand, so the compiler should assume it will be overwritten. I've tested it both with clang and gcc, and indeed it does pushl %ebx before the assembly: https://godbolt.org/z/Tchze37ja

On top of that, you're effectively clobbering %ebx set by cpuid, so p->ebx ends up containing "junk" prior to cpuid call rather than the value returned by cpuid.

@mgorny
Copy link
Member

mgorny commented Mar 16, 2024

Adding a revert on top of dbbdee2 (this is the commit I've originally tested) makes all tests pass again.

@devnexen
Copy link
Member Author

ok let s revert it then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openmp:libomp OpenMP host runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants