Skip to content

[X86][compiler-rt] Split CPU names even they have the same subtype #118237

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

Jump to bottom
Merged
merged 2 commits into from
Dec 2, 2024
Merged

[X86][compiler-rt] Split CPU names even they have the same subtype #118237

merged 2 commits into from
Dec 2, 2024

Conversation

phoebewang
Copy link
Contributor

Fixes: #118205

@phoebewang phoebewang mentioned this pull request Dec 2, 2024
Closed
boomanaiden154
boomanaiden154 reviewed Dec 2, 2024
View reviewed changes
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

llvm/lib/TargetParser/Host.cpp needs to be updated as well and is probably what is causing the linked issue.

As a side note, can I get some reviews on my PRs to test/merge this infra between compiler-rt and LLVM?

  1. [compiler-rt][X86] Unify getIntelProcessorTypeAndSubtype #97861
  2. [compiler-rt][X86] Better unify ProcessorFeatures and X86TargetParser #97856
  3. [compiler-rt][X86] Unify getAvailableFeatures #97872 (probably not ready for review yet).
  4. [compiler-rt] Add infrastructure for testing cpuid builtins #101927

That will make changes like this in the future easier.

@phoebewang
Copy link
Contributor Author

llvm/lib/TargetParser/Host.cpp needs to be updated as well and is probably what is causing the linked issue.

As a side note, can I get some reviews on my PRs to test/merge this infra between compiler-rt and LLVM?

  1. [compiler-rt][X86] Unify getIntelProcessorTypeAndSubtype #97861
  2. [compiler-rt][X86] Better unify ProcessorFeatures and X86TargetParser #97856
  3. [compiler-rt][X86] Unify getAvailableFeatures #97872 (probably not ready for review yet).
  4. [compiler-rt] Add infrastructure for testing cpuid builtins #101927

That will make changes like this in the future easier.

Good catch, thanks! I have a few comments on them, but I don't see how they guarantee synchronization between compiler and compiler-rt. Plus, how to sync with different libgcc?

@boomanaiden154
Copy link
Contributor

I don't see how they guarantee synchronization between compiler and compiler-rt

The plan is to make the files actually match given there are a large number of differences between them, through those PRs. Once everything matches appropriately, my plan is to write up another patch moving the implementations to .inc files that get diffed in the compiler-rt tests, so we get test failures if they're different. We might be able to directly copy over the code, but I think there were some issues with standalone builds, and that setup follows past precedent.

how to sync with different libgcc?

What do you mean by this? This shouldn't change any compiler-rt/libgcc compatibility guarantees (although it will probably catch issues where we don't maintain ABI compatibility with libgcc in compiler-rt), just make the code between LLVM and compiler-rt closer.

boomanaiden154
boomanaiden154 approved these changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

@phoebewang phoebewang merged commit 3348b46 into llvm:main Dec 2, 2024
8 checks passed
@phoebewang phoebewang deleted the pr118205 branch December 2, 2024 10:51
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 2, 2024

LLVM Buildbot has detected a new failure on builder libc-x86_64-debian-gcc-fullbuild-dbg running on libc-x86_64-debian-fullbuild while building compiler-rt,llvm at step 4 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/131/builds/11482

Here is the relevant piece of the build log for the reference 
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure)
...
[       OK ] LlvmLibcRemapFilePagesTest.ErrorInvalidAddress (3 us)
Ran 3 tests.  PASS: 3  FAIL: 0
[936/1102] Running unit test libc.test.src.sys.mman.linux.shm_test
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcShmTest.Basic
[       OK ] LlvmLibcShmTest.Basic (158 us)
[ RUN      ] LlvmLibcShmTest.NameConversion
[       OK ] LlvmLibcShmTest.NameConversion (79 us)
Ran 2 tests.  PASS: 2  FAIL: 0
[937/1102] Running unit test libc.test.src.sys.mman.linux.process_mrelease_test
FAILED: projects/libc/test/src/sys/mman/linux/CMakeFiles/libc.test.src.sys.mman.linux.process_mrelease_test /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/mman/linux/CMakeFiles/libc.test.src.sys.mman.linux.process_mrelease_test 
cd /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/mman/linux && /home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/build/projects/libc/test/src/sys/mman/linux/libc.test.src.sys.mman.linux.process_mrelease_test.__build__
[==========] Running 3 tests from 1 test suite.
[ RUN      ] LlvmLibcProcessMReleaseTest.NoError
/home/llvm-libc-buildbot/buildbot-worker/libc-x86_64-debian-fullbuild/libc-x86_64-debian-gcc-fullbuild-dbg/llvm-project/libc/test/src/sys/mman/linux/process_mrelease_test.cpp:44: FAILURE
Failed to match LIBC_NAMESPACE::process_mrelease(pidfd, 0) against Succeeds().
Expected return value to be equal to 0 but got -1.
Expected errno to be equal to "Success" but got "No such process".
[  FAILED  ] LlvmLibcProcessMReleaseTest.NoError
[ RUN      ] LlvmLibcProcessMReleaseTest.ErrorNotKilled
[       OK ] LlvmLibcProcessMReleaseTest.ErrorNotKilled (216 us)
[ RUN      ] LlvmLibcProcessMReleaseTest.ErrorNonExistingPidfd
[       OK ] LlvmLibcProcessMReleaseTest.ErrorNonExistingPidfd (11 us)
Ran 3 tests.  PASS: 2  FAIL: 1
[938/1102] Running unit test libc.test.src.sys.random.linux.getrandom_test
[==========] Running 4 tests from 1 test suite.
[ RUN      ] LlvmLibcGetRandomTest.InvalidFlag
[       OK ] LlvmLibcGetRandomTest.InvalidFlag (4 us)
[ RUN      ] LlvmLibcGetRandomTest.InvalidBuffer
[       OK ] LlvmLibcGetRandomTest.InvalidBuffer (6 us)
[ RUN      ] LlvmLibcGetRandomTest.ReturnsSize
[       OK ] LlvmLibcGetRandomTest.ReturnsSize (16 us)
[ RUN      ] LlvmLibcGetRandomTest.CheckValue
[       OK ] LlvmLibcGetRandomTest.CheckValue (5 us)
Ran 4 tests.  PASS: 4  FAIL: 0
[939/1102] Running unit test libc.test.src.sys.random.linux.getrandom_test.__NO_MISC_MATH_BASIC_OPS_OPT
[==========] Running 4 tests from 1 test suite.
[ RUN      ] LlvmLibcGetRandomTest.InvalidFlag
[       OK ] LlvmLibcGetRandomTest.InvalidFlag (4 us)
[ RUN      ] LlvmLibcGetRandomTest.InvalidBuffer
[       OK ] LlvmLibcGetRandomTest.InvalidBuffer (6 us)
[ RUN      ] LlvmLibcGetRandomTest.ReturnsSize
[       OK ] LlvmLibcGetRandomTest.ReturnsSize (16 us)
[ RUN      ] LlvmLibcGetRandomTest.CheckValue
[       OK ] LlvmLibcGetRandomTest.CheckValue (5 us)
Ran 4 tests.  PASS: 4  FAIL: 0
[940/1102] Running unit test libc.test.src.sys.random.linux.getrandom_test.__NO_FMA_OPT
[==========] Running 4 tests from 1 test suite.
[ RUN      ] LlvmLibcGetRandomTest.InvalidFlag

@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 2, 2024

LLVM Buildbot has detected a new failure on builder openmp-s390x-linux running on systemz-1 while building compiler-rt,llvm at step 6 "test-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/5228

Here is the relevant piece of the build log for the reference 
Step 6 (test-openmp) failure: test (failure)
******************** TEST 'libomp :: tasking/issue-94260-2.c' FAILED ********************
Exit Code: -11

Command Output (stdout):
--
# RUN: at line 1
/home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp   -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic && /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/./bin/clang -fopenmp -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test -L /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -fno-omit-frame-pointer -mbackchain -I /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/ompt /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic
# executed command: /home/uweigand/sandbox/buildbot/openmp-s390x-linux/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11

--

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boomanaiden154 boomanaiden154 boomanaiden154 approved these changes

@RKSimon RKSimon Awaiting requested review from RKSimon

@FreddyLeaf FreddyLeaf Awaiting requested review from FreddyLeaf

Assignees
No one assigned
Labels
compiler-rt:builtins compiler-rt
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Raptor Lake CPU detected as Alder Lake
4 participants
@phoebewang @boomanaiden154 @llvm-ci @llvmbot