Page MenuHomePhabricator

[CSSPGO] Passing the clang driver switch -fpseudo-probe-for-profiling to the linker.
ClosedPublic

Authored by hoy on Jan 22 2021, 3:53 PM.

Diff Detail

Event Timeline

hoy created this revision.Jan 22 2021, 3:53 PM
hoy requested review of this revision.Jan 22 2021, 3:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2021, 3:53 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
hoy updated this revision to Diff 318714.Jan 22 2021, 6:25 PM

Fixing test failure on Windows.

wenlei added inline comments.Jan 23 2021, 8:07 AM
clang/lib/Driver/ToolChains/CommonArgs.cpp
609

getLastArg(options::OPT_fpseudo_probe_for_profiling, options::OPT_fno_pseudo_probe_for_profiling instead?

hoy added inline comments.Jan 24 2021, 4:19 PM
clang/lib/Driver/ToolChains/CommonArgs.cpp
609

Good point, changed to

Args.hasFlag(options::OPT_fpseudo_probe_for_profiling, options::OPT_fno_pseudo_probe_for_profiling)

which matches the last positive flag

bool ArgList::hasFlag(OptSpecifier Pos, OptSpecifier Neg, bool Default) const {
  if (Arg *A = getLastArg(Pos, Neg))
    return A->getOption().matches(Pos);
  return Default;
}
hoy updated this revision to Diff 318878.Jan 24 2021, 4:19 PM

Addressing Wenleis' feedback.

wenlei accepted this revision.Jan 24 2021, 10:50 PM

lgtm, thanks.

This revision is now accepted and ready to land.Jan 24 2021, 10:50 PM
hoy updated this revision to Diff 320680.Feb 1 2021, 10:46 PM

Rebasing.

wmi added inline comments.Feb 1 2021, 11:19 PM
clang/lib/Driver/ToolChains/CommonArgs.cpp
609–610

The third arg of Args.hasFlag defaults to true, which means -plugin-opt=pseudo-probe-for-profiling will be inserted even if no -fpseudo-probe-for-profiling or -fno-pseudo-probe-for-profiling is provided. Is it expected?

clang/test/Driver/pseudo-probe-lto.c
7

Better add another check of NOPROBE when neither -fpseudo-probe-for-profiling or -fno-pseudo-probe-for-profiling is provided.

hoy added inline comments.Feb 1 2021, 11:25 PM
clang/lib/Driver/ToolChains/CommonArgs.cpp
609–610

Good catch, it's not expected. Thanks!

hoy updated this revision to Diff 320686.Feb 1 2021, 11:26 PM

Addressing Wei's comment.

hoy marked an inline comment as done.Feb 1 2021, 11:27 PM
hoy added inline comments.
clang/test/Driver/pseudo-probe-lto.c
7

Good point, test case added.

wmi accepted this revision.Tue, Feb 2, 9:21 AM

LGTM.

This revision was landed with ongoing or failed builds.Tue, Feb 2, 9:44 AM
This revision was automatically updated to reflect the committed changes.
hoy marked an inline comment as done.