Page MenuHomePhabricator

clang: Add -fcoverage-prefix-map
Needs ReviewPublic

Authored by keith on Jul 3 2020, 10:28 PM.

Details

Reviewers
vsk
arphaman
rnk
Summary

This flag allows you to re-write absolute paths in coverage data analogous to -fdebug-prefix-map. This flag is also implied by -ffile-prefix-map.

Diff Detail

Unit TestsFailed

TimeTest
4,190 mslinux > libomp.env::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/ompt /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/env/kmp_set_dispatch_buf.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/env/Output/kmp_set_dispatch_buf.c.tmp -lm -latomic && env KMP_DISP_NUM_BUFFERS=0 /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/env/Output/kmp_set_dispatch_buf.c.tmp
1,160 mslinux > libomp.worksharing/for::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/ompt /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/worksharing/for/kmp_set_dispatch_buf.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/worksharing/for/Output/kmp_set_dispatch_buf.c.tmp -lm -latomic && /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/worksharing/for/Output/kmp_set_dispatch_buf.c.tmp 7

Event Timeline

keith created this revision.Jul 3 2020, 10:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2020, 10:28 PM
Herald added a subscriber: dexonsmith. · View Herald Transcript
keith added a comment.Jul 3 2020, 10:31 PM

Open question: I don't know how all the toolchains fit together, but I noticed that only Clang.cpp handles -fmacro-prefix-map, but Clang.cpp, FreeBSD.cpp, and Gnu.cpp all handle -fdebug-prefix-map. I wasn't sure which pattern I should follow here, so right now this only adds the handling to Clang.cpp, please let me know if that's not appropriate in this case!

keith updated this revision to Diff 275506.Jul 4 2020, 9:57 AM

Fix tests on Windows

MaskRay added a subscriber: MaskRay.EditedMon, Jul 6, 6:12 PM

GCC obtained -fdebug-prefix-map first, then r256847 added both -fmacro-prefix-map and -ffile-prefix-map. GCC doesn't have -fcoverage-prefix-map and I saw your https://github.com/apple/swift/pull/32416
However, do we really need one new -f*-prefix-map for every feature?

If you think -fcoverage-prefix-map= is really necessary, you can probably ask whether GCC can adopt a -fcoverage-prefix-map for their gcov (.gcno files). If they do, reviewers may have more confidence to say that we should have the option as well.

keith added a comment.Mon, Jul 6, 6:15 PM

I originally implemented this behavior behind -fdebug-prefix-map but there was some pushback, more context: https://lists.llvm.org/pipermail/cfe-dev/2020-June/065668.html

-fdebug-prefix-map does not make sense to me because coverage is not debug info.
I am on the fence whether we need -fcoverage-prefix-map. I created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96092 (Should --coverage respect -ffile-prefix-map?)

clang/include/clang/Driver/Options.td
2066

Drop CC1AsOption

The option does not affect -cc1as (assembly input)

phosek added a subscriber: phosek.Sat, Jul 11, 5:20 PM

-fdebug-prefix-map does not make sense to me because coverage is not debug info.
I am on the fence whether we need -fcoverage-prefix-map. I created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96092 (Should --coverage respect -ffile-prefix-map?)

Looks like GCC has [-fprofile-prefix-path](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96092#c2) in the queue which is trying to achieve a similar thing. I'd prefer -fprofile-prefix-map to be consistent with the existing -f*-prefix-map options, both in terms of spelling and usage. I think that -fprofile-prefix-map is better than -fcoverage-prefix-map because it makes it clear that it applies -fprofile-* set of options.

-fdebug-prefix-map does not make sense to me because coverage is not debug info.
I am on the fence whether we need -fcoverage-prefix-map. I created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96092 (Should --coverage respect -ffile-prefix-map?)

Looks like GCC has [-fprofile-prefix-path](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96092#c2) in the queue which is trying to achieve a similar thing. I'd prefer -fprofile-prefix-map to be consistent with the existing -f*-prefix-map options, both in terms of spelling and usage. I think that -fprofile-prefix-map is better than -fcoverage-prefix-map because it makes it clear that it applies -fprofile-* set of options.

-fprofile-prefix-map looks good to me. I made another comment and CCed the author of https://gcc.gnu.org/git/?p=gcc.git&a=commit;h=44b326839d864fc10c459916abcc97f35a9ac3de (which added -fprofile-prefix-path) in that bug.