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

[ctx_profile] Integration test #92456

Merged
merged 5 commits into from
May 17, 2024
Merged

[ctx_profile] Integration test #92456

merged 5 commits into from
May 17, 2024

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented May 16, 2024

Compile with clang a program that's instrumented for contextual profiling and verify a profile can be collected.

Copy link

github-actions bot commented May 16, 2024

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

Compile with clang a program that's instrumented for contextual profiling
and verify a profile can be collected.
}
}

// CHECK: Guid: 11065787667334760794
Copy link
Contributor

Choose a reason for hiding this comment

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

A dump drive-by question, I wonder if contextual profile contain block counters? If yes maybe check them as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! And the test is then more interesting, too. Ptal.

// CHECK: check 2
// CHECK: check 2
// CHECK: check odd
// CHECK: check even
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there is another check odd line (corresponding to the 2nd iteration in the loop)?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup, tightened it.

Copy link
Contributor

@snehasish snehasish left a comment

Choose a reason for hiding this comment

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

lgtm


// block inlining because the pre-inliner otherwise will inline this - it's
// too small.
__attribute__((noinline)) void the_root() {
Copy link
Contributor

Choose a reason for hiding this comment

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

theRoot to be consistent with someFunction.

@mtrofin mtrofin merged commit 487d5af into llvm:main May 17, 2024
3 of 4 checks passed
@dyung
Copy link
Collaborator

dyung commented May 17, 2024

Hi @mtrofin, the test you added seems to be failing on a build bot: https://lab.llvm.org/buildbot/#/builders/247/builds/18559

Can you take a look?

@mtrofin
Copy link
Member Author

mtrofin commented May 17, 2024

Hi @mtrofin, the test you added seems to be failing on a build bot: https://lab.llvm.org/buildbot/#/builders/247/builds/18559

Can you take a look?

Sorry - was afk. Reverting.

boomanaiden154 added a commit that referenced this pull request May 18, 2024
mtrofin added a commit that referenced this pull request May 18, 2024
This reverts commit 881f20e.

Passing -ldl -lpthread explicitly
@mgorny
Copy link
Member

mgorny commented May 18, 2024

This test fails for me, at least when doing a standalone build:

FAIL: CtxProfile-x86_64-linux :: TestCases/generate-context.cpp (2185 of 7045)
******************** TEST 'CtxProfile-x86_64-linux :: TestCases/generate-context.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 4: mkdir -p /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X8
6_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include
+ mkdir -p /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include
RUN: at line 5: cp /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/llvm/include/llvm/ProfileData/CtxInstrContextNode.h /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include/
+ cp /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/llvm/include/llvm/ProfileData/CtxInstrContextNode.h /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include/
RUN: at line 8: /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/lib/llvm/19/bin/clang  --driver-mode=g++ -ldl -lpthread /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt/test/ctx_profile/TestCases/generate-context.cpp -lclang_rt.ctx_profile -I/var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include -O2 -o /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp.bin -mllvm -profile-context-root=theRoot
+ /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/lib/llvm/19/bin/clang --driver-mode=g++ -ldl -lpthread /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt/test/ctx_profile/TestCases/generate-context.cpp -lclang_rt.ctx_profile -I/var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include -O2 -o /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp.bin -mllvm -profile-context-root=theRoot
x86_64-pc-linux-gnu-ld: error: unable to find library -lclang_rt.ctx_profile
clang: error: linker command failed with exit code 1 (use -v to see invocation)

--

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

The library's at:

/var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/lib/clang/19/lib/linux/libclang_rt.ctx_profile-x86_64.a

So I guess it's missing the suffix and/or -L?

@mtrofin
Copy link
Member Author

mtrofin commented May 18, 2024

This test fails for me, at least when doing a standalone build:

FAIL: CtxProfile-x86_64-linux :: TestCases/generate-context.cpp (2185 of 7045)
******************** TEST 'CtxProfile-x86_64-linux :: TestCases/generate-context.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 4: mkdir -p /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X8
6_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include
+ mkdir -p /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include
RUN: at line 5: cp /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/llvm/include/llvm/ProfileData/CtxInstrContextNode.h /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include/
+ cp /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/llvm/include/llvm/ProfileData/CtxInstrContextNode.h /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include/
RUN: at line 8: /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/lib/llvm/19/bin/clang  --driver-mode=g++ -ldl -lpthread /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt/test/ctx_profile/TestCases/generate-context.cpp -lclang_rt.ctx_profile -I/var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include -O2 -o /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp.bin -mllvm -profile-context-root=theRoot
+ /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/lib/llvm/19/bin/clang --driver-mode=g++ -ldl -lpthread /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt/test/ctx_profile/TestCases/generate-context.cpp -lclang_rt.ctx_profile -I/var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp_include -O2 -o /var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/test/ctx_profile/X86_64LinuxConfig/TestCases/Output/generate-context.cpp.tmp.bin -mllvm -profile-context-root=theRoot
x86_64-pc-linux-gnu-ld: error: unable to find library -lclang_rt.ctx_profile
clang: error: linker command failed with exit code 1 (use -v to see invocation)

--

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

The library's at:

/var/tmp/portage/sys-libs/compiler-rt-sanitizers-19.0.0_pre20240518/work/compiler-rt_build/lib/clang/19/lib/linux/libclang_rt.ctx_profile-x86_64.a

So I guess it's missing the suffix and/or -L?

Can you give more context - is this on a bot, or can you give me the cmake invocation - for example, if I do cmake -GNinja -DCMAKE_BUILD_TYPE=Release ../llvm -DLLVM_ENABLE_PROJECTS="clang;compiler-rt", then ninja check-ctx_profile, the test passes fine and the library is under $MY_BUILD_DIR/lib/clang/19/lib/x86_64-unknown-linux-gnu/libclang_rt.ctx_profile.a - so no suffix. I'm trying to understand what would make it have a suffix, basically.

Thanks!

@mgorny
Copy link
Member

mgorny commented May 18, 2024

This is a standalone build against installed LLVM + Clang. Once you install these two, the absolute minimum is roughly:

$ cmake -G Ninja <path-to-source>/compiler-rt -DCOMPILER_RT_INCLUDE_TESTS=ON
$ ninja check-ctx_profile 

To get tests working, you'd also have to install the libraries onto the system. However, this is enough to reproduce the immediate problem.

@sscalpone
Copy link
Contributor

sscalpone commented May 18, 2024

Using -DCOMPILER_RT_BUILD_CTX_PROFILE=OFF seems like the correct solution. That's working for me.

Please test with CMake variable -DCOMPILER_RT_BUILD_SANITIZERS=OFF.

CMake Error at cmake/Modules/AddCompilerRT.cmake:355 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommon.x86_64>

  Objects of target "RTSanitizerCommon.x86_64" referenced but no such target
  exists.
Call Stack (most recent call first):
  lib/ctx_profile/CMakeLists.txt:22 (add_compiler_rt_runtime)


CMake Error at cmake/Modules/AddCompilerRT.cmake:355 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommon.x86_64>

  Objects of target "RTSanitizerCommon.x86_64" referenced but no such target
  exists.
Call Stack (most recent call first):
  lib/ctx_profile/CMakeLists.txt:22 (add_compiler_rt_runtime)


CMake Error at cmake/Modules/AddCompilerRT.cmake:355 (add_library):
  Error evaluating generator expression:

    $<TARGET_OBJECTS:RTSanitizerCommon.x86_64>

  Objects of target "RTSanitizerCommon.x86_64" referenced but no such target
  exists.
Call Stack (most recent call first):
  lib/ctx_profile/CMakeLists.txt:22 (add_compiler_rt_runtime)


CMake Error at cmake/Modules/AddCompilerRT.cmake:355 (add_library):
  No SOURCES given to target: clang_rt.ctx_profile-x86_64
Call Stack (most recent call first):
  lib/ctx_profile/CMakeLists.txt:22 (add_compiler_rt_runtime)

CMake Generate step failed.  Build files cannot be regenerated correctly.

@mtrofin
Copy link
Member Author

mtrofin commented May 18, 2024

@mgorny on a clean debian bookworm docker image, with cmake 3.25.1, I did this:

(monorepo)

mkdir build && cd build
cmake -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="clang;compiler-rt" ../llvm
sudo ninja install

this installed /usr/local/lib/clang/19/lib/x86_64-unknown-linux-gnu/libclang_rt.ctx_profile.a - so still no suffix.

Could you share more details on how you build and install, they appear to be essential to being able to repro? Thanks!

@dyung
Copy link
Collaborator

dyung commented May 18, 2024

Using -DCOMPILER_RT_BUILD_CTX_PROFILE=OFF seems like the correct solution. That's working for me.

Please test with CMake variable -DCOMPILER_RT_BUILD_SANITIZERS=OFF.

We are seeing the same cmake errors in our internal build where we set -DCOMPILER_RT_BUILD_SANITIZERS=OFF. Should we now be setting -DCOMPILER_RT_BUILD_CTX_PROFILE=OFF, or can the change be updated to work correctly when -DCOMPILER_RT_BUILD_SANITIZERS=OFF is defined?

@mgorny
Copy link
Member

mgorny commented May 18, 2024

@mgorny on a clean debian bookworm docker image, with cmake 3.25.1, I did this:

I guess you may need a proper multilib system (lib+lib64) system for that. Debian uses a custom multiarch, I suppose that could cause this.

Could you share more details on how you build and install, they appear to be essential to being able to repro? Thanks!

I'm using Gentoo Linux amd64. It's literally ACCEPT_KEYWORDS=** emerge -1v compiler-rt-sanitizers.

@mgorny
Copy link
Member

mgorny commented May 18, 2024

I'm going to try to find a simpler reproducer.

@mtrofin
Copy link
Member Author

mtrofin commented May 18, 2024

I'm going to try to find a simpler reproducer.

Thanks - I want to make sure I understand what substitutions to add (likely that's what I need to do in the lit.config.py)

But! picking up from @dyung and @sscalpone's comments: if you are already doing -DCOMPILER_RT_BUILD_SANITIZERS=OFF and/or -DCOMPILER_RT_BUILD_MEMPROF=OFF then the right solution for you should be what @sscalpone suggested, indeed.

@dyung
Copy link
Collaborator

dyung commented May 18, 2024

I'm going to try to find a simpler reproducer.

Thanks - I want to make sure I understand what substitutions to add (likely that's what I need to do in the lit.config.py)

But! picking up from @dyung and @sscalpone's comments: if you are already doing -DCOMPILER_RT_BUILD_SANITIZERS=OFF and/or -DCOMPILER_RT_BUILD_MEMPROF=OFF then the right solution for you should be what @sscalpone suggested, indeed.

Thanks for confirming. I can add the define to our build, but would greatly prefer if the build would check whether all of the required components are being built and only then enable it.

@mgorny
Copy link
Member

mgorny commented May 19, 2024

I'm going to try to find a simpler reproducer.

Thanks - I want to make sure I understand what substitutions to add (likely that's what I need to do in the lit.config.py)

Ok, could you try if you can reproduce if you do a monorepo build with:

cmake -G Ninja <source>/llvm -DCMAKE_BUILD_TYPE=RelWithDebInfo -DLLVM_CCACHE_BUILD=ON -DLLVM_TARGETS_TO_BUILD=host -DLLVM_ENABLE_PROJECTS='clang;compiler-rt' -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF

(-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF should be the key option, the rest is just my "quick build" defaults)

chapuni added a commit that referenced this pull request May 19, 2024
`__sanitizer_siginfo` has been introduced in D142117.
(llvmorg-16-init-17950-ged9ef9b4f248)
It is incompatible to -pedantic.

`clang_rt.ctx_profile` has been introduced in #92456.
@mtrofin
Copy link
Member Author

mtrofin commented May 19, 2024

I'm going to try to find a simpler reproducer.

Thanks - I want to make sure I understand what substitutions to add (likely that's what I need to do in the lit.config.py)

Ok, could you try if you can reproduce if you do a monorepo build with:

cmake -G Ninja <source>/llvm -DCMAKE_BUILD_TYPE=RelWithDebInfo -DLLVM_CCACHE_BUILD=ON -DLLVM_TARGETS_TO_BUILD=host -DLLVM_ENABLE_PROJECTS='clang;compiler-rt' -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF

(-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF should be the key option, the rest is just my "quick build" defaults)

ninja check-ctx_profile works with PR #92679 applied.

qiaojbao pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Jun 5, 2024
…3d19bf0f1

Local branch amd-gfx cbb3d19 Merged main:fe2ff54590c313551e7968179b48988ff0916290 into amd-gfx:57c0f5e83155
Remote branch main 881f20e Revert "[ctx_profile] Integration test (llvm#92456)"
Nanotwerp added a commit to Nanotwerp/nixpkgs that referenced this pull request Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants