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

Build instruction profiler runtime as part of compiler-rt #42433

Merged
merged 17 commits into from Jun 14, 2017

Conversation

Projects
None yet
@marco-c
Contributor

marco-c commented Jun 4, 2017

r? @alexcrichton

This is #38608 with some fixes.

Still missing:

  • testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);
  • enabling distribution (on which builders?);
  • documentation.

whitequark and others added some commits Feb 13, 2017

Add profiling support, through the rustc -Z profile flag.
When -Z profile is passed, the GCDAProfiling LLVM pass is added
to the pipeline, which uses debug information to instrument the IR.
After compiling with -Z profile, the $(OUT_DIR)/$(CRATE_NAME).gcno
file is created, containing initial profiling information.
After running the program built, the $(OUT_DIR)/$(CRATE_NAME).gcda
file is created, containing branch counters.

The created *.gcno and *.gcda files can be processed using
the "llvm-cov gcov" and "lcov" tools. The profiling data LLVM
generates does not faithfully follow the GCC's format for *.gcno
and *.gcda files, and so it will probably not work with other tools
(such as gcov itself) that consume these files.
@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive Jun 4, 2017

Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

Collaborator

rust-highfive commented Jun 4, 2017

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb
Member

eddyb commented Jun 5, 2017

@kennytm

This comment has been minimized.

Show comment
Hide comment
@kennytm

kennytm Jun 5, 2017

Member

I'm just wondering, aside from the name, is there any difference between #![sanitizer_runtime] and #![profiler_runtime], that leads to a distinct set of features?

Member

kennytm commented Jun 5, 2017

I'm just wondering, aside from the name, is there any difference between #![sanitizer_runtime] and #![profiler_runtime], that leads to a distinct set of features?

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jun 5, 2017

Member

Looking great, thanks for picking this up @marco-c!

testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);

I think this is supported on all platforms, right? If that's the case you can add --enable-sanitizers here. If not you can selectively enable it in particular builders, for example for x86_64-gnu you'd enable it here next to --enable-sanitizers. For OSX you'd do it here and on Windows you'd do it here.

enabling distribution (on which builders?)

This sort of depends on the point above. If we want to enable this on all platforms then src/ci/run.sh will do the trick. If not let's work out which builders need to distribute the profiler and we can figure that out.

documentation.

FWIW I'm not thinking anything fancy here, just a "hello world" documentation thing inside of src/doc/unstable-book/src/compiler-flags/profile.md.

Member

alexcrichton commented Jun 5, 2017

Looking great, thanks for picking this up @marco-c!

testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);

I think this is supported on all platforms, right? If that's the case you can add --enable-sanitizers here. If not you can selectively enable it in particular builders, for example for x86_64-gnu you'd enable it here next to --enable-sanitizers. For OSX you'd do it here and on Windows you'd do it here.

enabling distribution (on which builders?)

This sort of depends on the point above. If we want to enable this on all platforms then src/ci/run.sh will do the trick. If not let's work out which builders need to distribute the profiler and we can figure that out.

documentation.

FWIW I'm not thinking anything fancy here, just a "hello world" documentation thing inside of src/doc/unstable-book/src/compiler-flags/profile.md.

@marco-c

This comment has been minimized.

Show comment
Hide comment
@marco-c

marco-c Jun 5, 2017

Contributor

I've added some basic documentation, without going into detail into how to parse the gcno/gcda files, as I thought it was out of scope (if you think it isn't, I can describe in more detail how to generate a report using lcov and genhtml).

Contributor

marco-c commented Jun 5, 2017

I've added some basic documentation, without going into detail into how to parse the gcno/gcda files, as I thought it was out of scope (if you think it isn't, I can describe in more detail how to generate a report using lcov and genhtml).

@marco-c

This comment has been minimized.

Show comment
Hide comment
@marco-c

marco-c Jun 6, 2017

Contributor

I'm just wondering, aside from the name, is there any difference between #![sanitizer_runtime] and #![profiler_runtime], that leads to a distinct set of features?

The profiler instruments the binary to collect coverage data, the sanitizers (there are many different sanitizers) are mostly used to detect memory/threading errors. The sanitizers can also be used to collect coverage data (https://clang.llvm.org/docs/SanitizerCoverage.html), but the output isn't compatible with gcov that is the most widely used tool.

Contributor

marco-c commented Jun 6, 2017

I'm just wondering, aside from the name, is there any difference between #![sanitizer_runtime] and #![profiler_runtime], that leads to a distinct set of features?

The profiler instruments the binary to collect coverage data, the sanitizers (there are many different sanitizers) are mostly used to detect memory/threading errors. The sanitizers can also be used to collect coverage data (https://clang.llvm.org/docs/SanitizerCoverage.html), but the output isn't compatible with gcov that is the most widely used tool.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jun 6, 2017

Member

Ah it looks like maybe this won't work with a system and/or older LLVM? Travis failed with:

[00:46:39] LLVM ERROR: IO failure on output stream.
Member

alexcrichton commented Jun 6, 2017

Ah it looks like maybe this won't work with a system and/or older LLVM? Travis failed with:

[00:46:39] LLVM ERROR: IO failure on output stream.
Show outdated Hide outdated src/doc/unstable-book/src/compiler-flags/profile.md
This feature allows the generation of code coverage reports.
Set the compiler flags `-Ccodegen-units=1 -Clink-dead-code -Cpasses=insert-gcov-profiling -Zno-landing-pads` to enable gcov profiling.

This comment has been minimized.

@alexcrichton

alexcrichton Jun 6, 2017

Member

Shouldn't -Z profile be mentioned here? If all these other flags are required, could -Z profile imply them?

@alexcrichton

alexcrichton Jun 6, 2017

Member

Shouldn't -Z profile be mentioned here? If all these other flags are required, could -Z profile imply them?

This comment has been minimized.

@marco-c

marco-c Jun 6, 2017

Contributor

Right, I think they aren't strictly required. I've replaced them with -Zprofile.

@marco-c

marco-c Jun 6, 2017

Contributor

Right, I think they aren't strictly required. I've replaced them with -Zprofile.

@kennytm

This comment has been minimized.

Show comment
Hide comment
@kennytm

kennytm Jun 6, 2017

Member

@marco-c Yes I understand the difference between sancov and gcov. But what I mean is, #![sanitizer_runtime] and #![profiler_runtime] only affect linker behavior ("link to this crate if -Zsanitizer=X/-Zprofile is used"). The reasons of split I can see are:

  • The profiler runtime is always a staticlib, while the sanitizer runtime are sometimes dylib
  • You can use -Zprofile and -Zsanitizer=X simultaneously
Member

kennytm commented Jun 6, 2017

@marco-c Yes I understand the difference between sancov and gcov. But what I mean is, #![sanitizer_runtime] and #![profiler_runtime] only affect linker behavior ("link to this crate if -Zsanitizer=X/-Zprofile is used"). The reasons of split I can see are:

  • The profiler runtime is always a staticlib, while the sanitizer runtime are sometimes dylib
  • You can use -Zprofile and -Zsanitizer=X simultaneously
@kennytm

This comment has been minimized.

Show comment
Hide comment
@kennytm

kennytm Jun 6, 2017

Member

@alexcrichton I think it is because /checkout is read-only on Docker. Those *.gcno and *.gcda are placed "next to the existing source" (/checkout/src/test/run-make/profile/), so LLVM failed by writing to a read-only drive.

Member

kennytm commented Jun 6, 2017

@alexcrichton I think it is because /checkout is read-only on Docker. Those *.gcno and *.gcda are placed "next to the existing source" (/checkout/src/test/run-make/profile/), so LLVM failed by writing to a read-only drive.

@marco-c

This comment has been minimized.

Show comment
Hide comment
@marco-c

marco-c Jun 6, 2017

Contributor

@alexcrichton I think it is because /checkout is read-only on Docker. Those *.gcno and *.gcda are placed "next to the existing source" (/checkout/src/test/run-make/profile/), so LLVM failed by writing to a read-only drive.

They should be placed in the output directory.
If you do:

mkdir target
rustc -g -Z profile main.rs --out-dir target

Both the binary and the gcno file will appear in the target directory.

The error does definitely seem related to failure in writing files though.

Contributor

marco-c commented Jun 6, 2017

@alexcrichton I think it is because /checkout is read-only on Docker. Those *.gcno and *.gcda are placed "next to the existing source" (/checkout/src/test/run-make/profile/), so LLVM failed by writing to a read-only drive.

They should be placed in the output directory.
If you do:

mkdir target
rustc -g -Z profile main.rs --out-dir target

Both the binary and the gcno file will appear in the target directory.

The error does definitely seem related to failure in writing files though.

@marco-c

This comment has been minimized.

Show comment
Hide comment
@marco-c

marco-c Jun 6, 2017

Contributor

Oh, wait a second, the builder has LLVM 3.7, but the three-element variant of the llvm.gcov metadata is only supported from LLVM 3.9.

@alexcrichton should I revert back to the old two-element variant that is also supported on LLVM < 3.9?
Or should a newer LLVM be used on the builders?

Contributor

marco-c commented Jun 6, 2017

Oh, wait a second, the builder has LLVM 3.7, but the three-element variant of the llvm.gcov metadata is only supported from LLVM 3.9.

@alexcrichton should I revert back to the old two-element variant that is also supported on LLVM < 3.9?
Or should a newer LLVM be used on the builders?

@whitequark

This comment has been minimized.

Show comment
Hide comment
@whitequark

whitequark Jun 6, 2017

Contributor

@marco-c see #38608 (comment) re: LLVM version...

Contributor

whitequark commented Jun 6, 2017

@marco-c see #38608 (comment) re: LLVM version...

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jun 6, 2017

Member

Ok in that case we won't want to blanket enable this across all builders (as this is known to fail on at least one).

Let's enable three testing builders for OSX/Windows/Linux as mentioned above and then let's similarly modify the builders that produce releases for these platforms. You can do that by modifying dist-i686-gnu and dist-x86_64-gnu docker images, 32-bit osx, 64-bit osx, and windows deployments

Member

alexcrichton commented Jun 6, 2017

Ok in that case we won't want to blanket enable this across all builders (as this is known to fail on at least one).

Let's enable three testing builders for OSX/Windows/Linux as mentioned above and then let's similarly modify the builders that produce releases for these platforms. You can do that by modifying dist-i686-gnu and dist-x86_64-gnu docker images, 32-bit osx, 64-bit osx, and windows deployments

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jun 6, 2017

Member

Can you change dist-x86_64-gnu and dist-i686-gnu in the src/ci/docker folder as well? (those are the Linux releases)

Member

alexcrichton commented Jun 6, 2017

Can you change dist-x86_64-gnu and dist-i686-gnu in the src/ci/docker folder as well? (those are the Linux releases)

@marco-c

This comment has been minimized.

Show comment
Hide comment
@marco-c

marco-c Jun 6, 2017

Contributor

Can you change dist-x86_64-gnu and dist-i686-gnu in the src/ci/docker folder as well? (those are the Linux releases)

I changed dist-x86_64-linux and dist-i686-linux, as dist-x86_64-gnu and dist-i686-gnu don't exist :)

Contributor

marco-c commented Jun 6, 2017

Can you change dist-x86_64-gnu and dist-i686-gnu in the src/ci/docker folder as well? (those are the Linux releases)

I changed dist-x86_64-linux and dist-i686-linux, as dist-x86_64-gnu and dist-i686-gnu don't exist :)

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jun 6, 2017

Member

Oops sorry I missed that! Yeah as-is is fine

Member

alexcrichton commented Jun 6, 2017

Oops sorry I missed that! Yeah as-is is fine

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jun 6, 2017

Member

Ok great! Let's see what @bors has to say...

@bors: r+

Member

alexcrichton commented Jun 6, 2017

Ok great! Let's see what @bors has to say...

@bors: r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 6, 2017

Contributor

📌 Commit 43eaa1b has been approved by alexcrichton

Contributor

bors commented Jun 6, 2017

📌 Commit 43eaa1b has been approved by alexcrichton

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Jun 6, 2017

@bors: r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 6, 2017

Contributor

📌 Commit 4c908a9 has been approved by alexcrichton

Contributor

bors commented Jun 6, 2017

📌 Commit 4c908a9 has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 7, 2017

Contributor

⌛️ Testing commit 4c908a9 with merge 96c7e75...

Contributor

bors commented Jun 7, 2017

⌛️ Testing commit 4c908a9 with merge 96c7e75...

bors added a commit that referenced this pull request Jun 7, 2017

Auto merge of #42433 - marco-c:profiling, r=alexcrichton
Build instruction profiler runtime as part of compiler-rt

r? @alexcrichton

This is #38608 with some fixes.

Still missing:
- [x] testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);
- [x] enabling distribution (on which builders?);
- [x] documentation.
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 7, 2017

Contributor

💔 Test failed - status-appveyor

Contributor

bors commented Jun 7, 2017

💔 Test failed - status-appveyor

@kennytm

This comment has been minimized.

Show comment
Hide comment
@kennytm

kennytm Jun 7, 2017

Member

x86_64-pc-windows-gnu:

cargo:warning=../compiler-rt/lib/profile\InstrProfilingUtil.c:26:25: fatal error: sys/utsname.h: No such file or directory
cargo:warning= #include <sys/utsname.h>
cargo:warning=                         ^

also, a lot of warnings like

cargo:warning=../compiler-rt/lib/profile\InstrProfiling.c: In function '__llvm_profile_get_magic':
cargo:warning=../compiler-rt/lib/profile\InstrProfiling.c:27:1: warning: visibility attribute not supported in this configuration; ignored [-Wattributes]
Member

kennytm commented Jun 7, 2017

x86_64-pc-windows-gnu:

cargo:warning=../compiler-rt/lib/profile\InstrProfilingUtil.c:26:25: fatal error: sys/utsname.h: No such file or directory
cargo:warning= #include <sys/utsname.h>
cargo:warning=                         ^

also, a lot of warnings like

cargo:warning=../compiler-rt/lib/profile\InstrProfiling.c: In function '__llvm_profile_get_magic':
cargo:warning=../compiler-rt/lib/profile\InstrProfiling.c:27:1: warning: visibility attribute not supported in this configuration; ignored [-Wattributes]
@marco-c

This comment has been minimized.

Show comment
Hide comment
@marco-c

marco-c Jun 7, 2017

Contributor

The build on Windows is failing because sys/utsname.h is being included (https://github.com/llvm-mirror/compiler-rt/blob/master/lib/profile/InstrProfilingUtil.c#L25), because COMPILER_RT_HAS_UNAME is defined (https://github.com/llvm-mirror/compiler-rt/blob/835a29632377870f0d7863f0101e1be441327476/lib/profile/InstrProfilingPort.h#L48).

I suppose we could avoid enabling the profiler on the GNU Windows targets.

Contributor

marco-c commented Jun 7, 2017

The build on Windows is failing because sys/utsname.h is being included (https://github.com/llvm-mirror/compiler-rt/blob/master/lib/profile/InstrProfilingUtil.c#L25), because COMPILER_RT_HAS_UNAME is defined (https://github.com/llvm-mirror/compiler-rt/blob/835a29632377870f0d7863f0101e1be441327476/lib/profile/InstrProfilingPort.h#L48).

I suppose we could avoid enabling the profiler on the GNU Windows targets.

@marco-c

This comment has been minimized.

Show comment
Hide comment
@marco-c

marco-c Jun 12, 2017

Contributor

I added WindowsMMap.c to the list of files to compile and defined strdup, open and fdopen as _strdup, _open and _fdopen.

Contributor

marco-c commented Jun 12, 2017

I added WindowsMMap.c to the list of files to compile and defined strdup, open and fdopen as _strdup, _open and _fdopen.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Jun 13, 2017

@bors: r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 13, 2017

Contributor

📌 Commit d7039fb has been approved by alexcrichton

Contributor

bors commented Jun 13, 2017

📌 Commit d7039fb has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 13, 2017

Contributor

⌛️ Testing commit d7039fb with merge 3181151...

Contributor

bors commented Jun 13, 2017

⌛️ Testing commit d7039fb with merge 3181151...

bors added a commit that referenced this pull request Jun 13, 2017

Auto merge of #42433 - marco-c:profiling, r=alexcrichton
Build instruction profiler runtime as part of compiler-rt

r? @alexcrichton

This is #38608 with some fixes.

Still missing:
- [x] testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);
- [x] enabling distribution (on which builders?);
- [x] documentation.
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 13, 2017

Contributor

💔 Test failed - status-travis

Contributor

bors commented Jun 13, 2017

💔 Test failed - status-travis

@kennytm

This comment has been minimized.

Show comment
Hide comment
@kennytm

kennytm Jun 13, 2017

Member

distcheck failed.

[01:23:38] Distcheck rust-src
[01:23:38] error: failed to load source for a dependency on `profiler_builtins`
[01:23:38] 
[01:23:38] Caused by:
[01:23:38]   Unable to update file:///checkout/obj/build/tmp/distcheck-src/rust-src/lib/rustlib/src/rust/src/libprofiler_builtins
[01:23:38] 
[01:23:38] Caused by:
[01:23:38]   failed to read `/checkout/obj/build/tmp/distcheck-src/rust-src/lib/rustlib/src/rust/src/libprofiler_builtins/Cargo.toml`
[01:23:38] 
[01:23:38] Caused by:
[01:23:38]   No such file or directory (os error 2)
Member

kennytm commented Jun 13, 2017

distcheck failed.

[01:23:38] Distcheck rust-src
[01:23:38] error: failed to load source for a dependency on `profiler_builtins`
[01:23:38] 
[01:23:38] Caused by:
[01:23:38]   Unable to update file:///checkout/obj/build/tmp/distcheck-src/rust-src/lib/rustlib/src/rust/src/libprofiler_builtins
[01:23:38] 
[01:23:38] Caused by:
[01:23:38]   failed to read `/checkout/obj/build/tmp/distcheck-src/rust-src/lib/rustlib/src/rust/src/libprofiler_builtins/Cargo.toml`
[01:23:38] 
[01:23:38] Caused by:
[01:23:38]   No such file or directory (os error 2)
@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Jun 13, 2017

@bors: r+

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 13, 2017

Contributor

📌 Commit 5c084fd has been approved by alexcrichton

Contributor

bors commented Jun 13, 2017

📌 Commit 5c084fd has been approved by alexcrichton

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 13, 2017

Contributor

⌛️ Testing commit 5c084fd with merge 6b2faa5...

Contributor

bors commented Jun 13, 2017

⌛️ Testing commit 5c084fd with merge 6b2faa5...

bors added a commit that referenced this pull request Jun 13, 2017

Auto merge of #42433 - marco-c:profiling, r=alexcrichton
Build instruction profiler runtime as part of compiler-rt

r? @alexcrichton

This is #38608 with some fixes.

Still missing:
- [x] testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);
- [x] enabling distribution (on which builders?);
- [x] documentation.

@kennytm kennytm referenced this pull request Jun 13, 2017

Closed

Support `-Zprofile` #10

3 of 3 tasks complete
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 14, 2017

Contributor

💔 Test failed - status-travis

Contributor

bors commented Jun 14, 2017

💔 Test failed - status-travis

@est31

This comment has been minimized.

Show comment
Hide comment
@est31

est31 Jun 14, 2017

Contributor
The job exceeded the maximum time limit for jobs, and has been terminated.

Maybe a retry will fix it.

@bors retry

Contributor

est31 commented Jun 14, 2017

The job exceeded the maximum time limit for jobs, and has been terminated.

Maybe a retry will fix it.

@bors retry

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 14, 2017

Contributor

⌛️ Testing commit 5c084fd with merge 7375046...

Contributor

bors commented Jun 14, 2017

⌛️ Testing commit 5c084fd with merge 7375046...

bors added a commit that referenced this pull request Jun 14, 2017

Auto merge of #42433 - marco-c:profiling, r=alexcrichton
Build instruction profiler runtime as part of compiler-rt

r? @alexcrichton

This is #38608 with some fixes.

Still missing:
- [x] testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);
- [x] enabling distribution (on which builders?);
- [x] documentation.
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 14, 2017

Contributor

💔 Test failed - status-appveyor

Contributor

bors commented Jun 14, 2017

💔 Test failed - status-appveyor

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton
Member

alexcrichton commented Jun 14, 2017

@bors: retry

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 14, 2017

Contributor

⌛️ Testing commit 5c084fd with merge 8af4502...

Contributor

bors commented Jun 14, 2017

⌛️ Testing commit 5c084fd with merge 8af4502...

bors added a commit that referenced this pull request Jun 14, 2017

Auto merge of #42433 - marco-c:profiling, r=alexcrichton
Build instruction profiler runtime as part of compiler-rt

r? @alexcrichton

This is #38608 with some fixes.

Still missing:
- [x] testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);
- [x] enabling distribution (on which builders?);
- [x] documentation.
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 14, 2017

Contributor

💔 Test failed - status-travis

Contributor

bors commented Jun 14, 2017

💔 Test failed - status-travis

@est31

This comment has been minimized.

Show comment
Hide comment
@est31

est31 Jun 14, 2017

Contributor

Spurious musl segfault (#38618)

@bors retry

Contributor

est31 commented Jun 14, 2017

Spurious musl segfault (#38618)

@bors retry

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 14, 2017

Contributor

⌛️ Testing commit 5c084fd with merge dfa7e21...

Contributor

bors commented Jun 14, 2017

⌛️ Testing commit 5c084fd with merge dfa7e21...

bors added a commit that referenced this pull request Jun 14, 2017

Auto merge of #42433 - marco-c:profiling, r=alexcrichton
Build instruction profiler runtime as part of compiler-rt

r? @alexcrichton

This is #38608 with some fixes.

Still missing:
- [x] testing with profiler enabled on some builders (on which ones? Should I add the option to some of the already existing configurations, or create a new configuration?);
- [x] enabling distribution (on which builders?);
- [x] documentation.
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Jun 14, 2017

Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing dfa7e21 to master...

Contributor

bors commented Jun 14, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing dfa7e21 to master...

@bors bors merged commit 5c084fd into rust-lang:master Jun 14, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@sylvestre

This comment has been minimized.

Show comment
Hide comment
@sylvestre

sylvestre Jun 15, 2017

Contributor

Clap clap! 👍

Contributor

sylvestre commented Jun 15, 2017

Clap clap! 👍

@ofek

This comment has been minimized.

Show comment
Hide comment
@ofek

ofek Jul 27, 2017

So now that this is merged, how can one test with coverage?

ofek commented Jul 27, 2017

So now that this is merged, how can one test with coverage?

@kennytm

This comment has been minimized.

Show comment
Hide comment
@kennytm

kennytm Jul 27, 2017

Member

@ofek Same as https://jbp.io/2017/07/19/measuring-test-coverage-of-rust-programs, but use -Zprofile instead of -Cpasses=insert-gcov-profiling -L/usr/lib/llvm-3.8/lib/clang/3.8.1/lib/linux/ -lclang_rt.profile-x86_64.

Member

kennytm commented Jul 27, 2017

@ofek Same as https://jbp.io/2017/07/19/measuring-test-coverage-of-rust-programs, but use -Zprofile instead of -Cpasses=insert-gcov-profiling -L/usr/lib/llvm-3.8/lib/clang/3.8.1/lib/linux/ -lclang_rt.profile-x86_64.

@ofek

This comment has been minimized.

Show comment
Hide comment
@ofek

ofek Jul 27, 2017

@kennytm Brilliant, thanks!

ofek commented Jul 27, 2017

@kennytm Brilliant, thanks!

@marco-c marco-c deleted the marco-c:profiling branch Mar 29, 2018

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