Page MenuHomePhabricator

[Clang] Map .gcda paths according to -fcoverage-prefix-map
Needs ReviewPublic

Authored by vit9696 on May 2 2022, 6:09 AM.

Diff Detail

Event Timeline

vit9696 created this revision.May 2 2022, 6:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2022, 6:09 AM
vit9696 requested review of this revision.May 2 2022, 6:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2022, 6:09 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
vit9696 updated this revision to Diff 426414.May 2 2022, 7:18 AM
keith added a comment.May 2 2022, 11:24 AM

Based on the issue it sounds like this should be gated behind a new -fprofile-prefix-map flag? I assume we'd also want -ffile-prefix-map to apply to it as well, similar to the others. And we'll definitely want some tests here!

Based on the issue it sounds like this should be gated behind a new -fprofile-prefix-map flag? I assume we'd also want -ffile-prefix-map to apply to it as well, similar to the others. And we'll definitely want some tests here!

For tests, sure. I will add some once there is general agreement on the option.

As for the new flag, I am confused. To me the timeline is as follows:

  • LLVM implemented `-fprofile-prefix-map.
  • GCC implemented -fprofile-prefix-map.
  • LLVM renamed -fprofile-prefix-map to -fcoverage-prefix-map.
  • GCC preliminary agreed to extend -fprofile-prefix-map with gcda path mapping.
  • This patch implements this extension to LLVM -fcoverage-prefix-map, which remains being renamed -fprofile-prefix-map.

Do I misunderstand something? These flags are rather confusing in their semantics.

keith added a comment.May 2 2022, 3:17 PM

I took this comment from the issue:

Since the feature you're proposing is specific to gcov, using a separate flag that matches the name used by GCC would be preferable to me.

To mean that we should introduce a new flag matching gcc's name. I think in general fewer flags would be preferred though, so I think it depends on if it would ever make sense to have separate values for these in the same build?

Got it. To be honest, I cannot imagine a situation where one would prefer to keep separate behaviour, and with GCC it would be exactly just one. In my view, if we introduce -fprofile-prefix-map, it should rather mirror the entire -fcoverage-prefix-map behaviour, not some parts of it.

marxin added a subscriber: marxin.May 11 2022, 4:24 AM