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

Add codegen test for PGO instrumentation. #60038

Merged
merged 6 commits into from
Apr 25, 2019

Conversation

michaelwoerister
Copy link
Member

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in rustc.

The second commit updates a test case to the new commandline option syntax introduced in #59874. Without the fix the test still works, but it confusingly creates a directory called test.profraw, which usually is the name of the file where profiling data is collected.

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 17, 2019
@michaelwoerister
Copy link
Member Author

cc PGO tracking issue: #59913

@michaelwoerister michaelwoerister changed the title Pgo updates 2 Add codegen test for PGO instrumentation. Apr 17, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:347de5ee:start=1555498908963532572,finish=1555499015314769160,duration=106351236588
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
travis_time:start:test_codegen
Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:16:26] 
[01:16:26] running 139 tests
[01:16:29] i..iii.....iii..iiii.....i....................i..i.................i.....i..........ii..F.i..i.ii... 100/139
[01:16:31] failures:
[01:16:31] 
[01:16:31] ---- [codegen] codegen/pgo-instrumentation.rs stdout ----
[01:16:31] 
[01:16:31] 
[01:16:31] error: compilation failed!
[01:16:31] status: exit code: 1
[01:16:31] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/codegen/pgo-instrumentation.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/pgo-instrumentation" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Z" "pgo-gen" "-Ccodegen-units=1" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/pgo-instrumentation/auxiliary" "--emit=llvm-ir"
[01:16:31] ------------------------------------------
[01:16:31] 
[01:16:31] ------------------------------------------
[01:16:31] stderr:
---
[01:16:31] thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:517:22
[01:16:31] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
[01:16:31] 
[01:16:31] 
[01:16:31] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/codegen" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "codegen" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:16:31] 
[01:16:31] 
[01:16:31] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:16:31] Build completed unsuccessfully in 0:11:58
[01:16:31] Build completed unsuccessfully in 0:11:58
[01:16:31] make: *** [check] Error 1
[01:16:31] Makefile:48: recipe for target 'check' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:007be4c8
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Apr 17 12:20:16 UTC 2019

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@alexcrichton
Copy link
Member

r=me with passing tests, thanks @michaelwoerister!

(I suspect that this may need to be ignored on older LLVM or something like that)

@michaelwoerister
Copy link
Member Author

(I suspect that this may need to be ignored on older LLVM or something like that)

Yes, that too probably. But here the test is failing because the profiler runtime isn't being built for this CI image. I'm trying to think of the best way of solving this (either extend compiletest with more ignore-xxx options or switch this to a run-make test)...

@alexcrichton
Copy link
Member

It might be nice to add some compiletest options which may not be too too bad if we want to add more tests in the future, in general run-make tests are a bit of a bummer to write

@michaelwoerister
Copy link
Member Author

I implemented the compiletest version as suggested.

@alexcrichton
Copy link
Member

@bors: r+

Nice!

@bors
Copy link
Contributor

bors commented Apr 18, 2019

📌 Commit cc77087 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 18, 2019
…lexcrichton

Add codegen test for PGO instrumentation.

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`.

The second commit updates a test case to the new commandline option syntax introduced in rust-lang#59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
Centril added a commit to Centril/rust that referenced this pull request Apr 18, 2019
…lexcrichton

Add codegen test for PGO instrumentation.

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`.

The second commit updates a test case to the new commandline option syntax introduced in rust-lang#59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
Centril added a commit to Centril/rust that referenced this pull request Apr 18, 2019
…lexcrichton

Add codegen test for PGO instrumentation.

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`.

The second commit updates a test case to the new commandline option syntax introduced in rust-lang#59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
Centril added a commit to Centril/rust that referenced this pull request Apr 18, 2019
…lexcrichton

Add codegen test for PGO instrumentation.

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`.

The second commit updates a test case to the new commandline option syntax introduced in rust-lang#59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
Centril added a commit to Centril/rust that referenced this pull request Apr 18, 2019
…lexcrichton

Add codegen test for PGO instrumentation.

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`.

The second commit updates a test case to the new commandline option syntax introduced in rust-lang#59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
Centril added a commit to Centril/rust that referenced this pull request Apr 19, 2019
…lexcrichton

Add codegen test for PGO instrumentation.

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`.

The second commit updates a test case to the new commandline option syntax introduced in rust-lang#59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
@Centril
Copy link
Contributor

Centril commented Apr 19, 2019

Failed in #60101 (comment), @bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 19, 2019
@michaelwoerister
Copy link
Member Author

Turns out that the branching in the sanitizer test Makefiles implements more complicated ignoring logic than just looking for sanitizer support. Trying to recreate that with compiletest directives. If that doesn't work, I'll have to bring the logic back.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 24, 2019

📌 Commit ff976fe has been approved by michaelwoerister

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2019
@michaelwoerister
Copy link
Member Author

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 24, 2019
@michaelwoerister
Copy link
Member Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Apr 24, 2019

📌 Commit ff976fe has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 25, 2019
…lexcrichton

Add codegen test for PGO instrumentation.

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`.

The second commit updates a test case to the new commandline option syntax introduced in rust-lang#59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
bors added a commit that referenced this pull request Apr 25, 2019
Rollup of 6 pull requests

Successful merges:

 - #59560 (MIR generation cleanup)
 - #59697 (tweak unresolved label suggestion)
 - #60038 (Add codegen test for PGO instrumentation.)
 - #60160 (Fix #58270, fix off-by-one error in error diagnostics.)
 - #60185 (Reexport IntErrorKind in std)
 - #60243 (Add regression test for #53249.)

Failed merges:

r? @ghost
@bors bors merged commit ff976fe into rust-lang:master Apr 25, 2019
@bors
Copy link
Contributor

bors commented Apr 25, 2019

⌛ Testing commit ff976fe with merge 9aea116...

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 11, 2023
Mark `___asan_globals_registered` as an exported symbol for LTO

Export a weak symbol defined by AddressSanitizer instrumentation.
Previously, when using LTO, the symbol would get internalized and eliminated.

Fixes rust-lang#113404.

---------------

FWIW, let me list similar PRs from the past + who reviewed them:

* rust-lang#68410 (fixing `__msan_keep_going` and `__msan_track_origins`; reviewed by `@varkor)`
* rust-lang#60038 and rust-lang#48346 (fixing `__llvm_profile_raw_version` and `__llvm_profile_filename`; reviewed by `@alexcrichton)`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 12, 2023
Mark `___asan_globals_registered` as an exported symbol for LTO

Export a weak symbol defined by AddressSanitizer instrumentation.
Previously, when using LTO, the symbol would get internalized and eliminated.

Fixes rust-lang#113404.

---------------

FWIW, let me list similar PRs from the past + who reviewed them:

* rust-lang#68410 (fixing `__msan_keep_going` and `__msan_track_origins`; reviewed by ``@varkor)``
* rust-lang#60038 and rust-lang#48346 (fixing `__llvm_profile_raw_version` and `__llvm_profile_filename`; reviewed by ``@alexcrichton)``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants