[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
anemet added a comment. Sorted these out in https://reviews.llvm.org/rL319576, https://reviews.llvm.org/rL319577 and https://reviews.llvm.org/rL319578. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
anemet added a comment. Looks like it's a test problem. When I tweak the sample profile file according to https://clang.llvm.org/docs/UsersManual.html#sample-profile-text-format, I do get hotness on the remarks. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
davide added a comment. In https://reviews.llvm.org/D34082#942420, @anemet wrote: > @modocache, @davide, are you guys sure this feature is working? The test > does not actually check whether hotness is included in the remarks and when I > run it manually they are missing. In https://reviews.llvm.org/D40678, I am > filtering out remarks with no hotness when any threshold is set all the > remarks are filtered out in this new test. > > So either the test is incorrect or somehow with sample-based profiling we > don't get hotness info. > > Any ideas? I am inclined to just remove this test for now and file a bug to > fix this in order to unblock https://reviews.llvm.org/D40678. I don't know, I haven't reviewed this feature, but I can take a look later today. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
anemet added a subscriber: davide. anemet added a comment. @modocache, @davide, are you guys sure this feature is working? The test does not actually check whether hotness is included in the remarks and when I run it manually they are missing. In https://reviews.llvm.org/D40678, I am filtering out remarks with no hotness when any threshold is set all the remarks are filtered out in this new test. So either the test is incorrect or somehow with sample-based profiling we don't get hotness info. Any ideas? I am inclined to just remove this test for now and file a bug to fix this in order to unblock https://reviews.llvm.org/D40678. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
modocache updated this revision to Diff 103647. modocache added a comment. Update the sampling profile text so that it produces the hotness expected by the test. This is ready to go :) https://reviews.llvm.org/D34082 Files: lib/Frontend/CompilerInvocation.cpp test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext test/Frontend/optimization-remark-with-hotness.c Index: test/Frontend/optimization-remark-with-hotness.c === --- test/Frontend/optimization-remark-with-hotness.c +++ test/Frontend/optimization-remark-with-hotness.c @@ -1,11 +1,21 @@ +// Generate instrumentation and sampling profile data. // RUN: llvm-profdata merge \ -// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ +// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ // RUN: -o %t.profdata +// RUN: llvm-profdata merge -sample \ +// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \ +// RUN: -o %t-sample.profdata +// // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ // RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ // RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \ // RUN: -Rpass-analysis=inline -Rpass-missed=inline \ // RUN: -fdiagnostics-show-hotness -verify +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ +// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ +// RUN: -fprofile-sample-use=%t-sample.profdata -Rpass=inline \ +// RUN: -Rpass-analysis=inline -Rpass-missed=inline \ +// RUN: -fdiagnostics-show-hotness -verify // The clang version of the previous test. // RUN: %clang -target x86_64-apple-macosx10.9 %s -c -emit-llvm -o /dev/null \ // RUN: -fprofile-instr-use=%t.profdata -Rpass=inline \ Index: test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext === --- /dev/null +++ test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext @@ -0,0 +1,7 @@ +foo:0:0 + 0: 0 +bar:29:29 + 6: foo:0 +main:0:0 + 0: 0 bar:0 + Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -892,14 +892,18 @@ Opts.DiagnosticsWithHotness = Args.hasArg(options::OPT_fdiagnostics_show_hotness); + bool UsingSampleProfile = !Opts.SampleProfileFile.empty(); + if (Opts.DiagnosticsWithHotness && - Opts.getProfileUse() == CodeGenOptions::ProfileNone) + Opts.getProfileUse() == CodeGenOptions::ProfileNone && + !UsingSampleProfile) { Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo); + } // If the user requested to use a sample profile for PGO, then the // backend will need to track source location information so the profile // can be incorporated into the IR. - if (!Opts.SampleProfileFile.empty()) + if (UsingSampleProfile) NeedLocTracking = true; // If the user requested a flag that requires source locations available in Index: test/Frontend/optimization-remark-with-hotness.c === --- test/Frontend/optimization-remark-with-hotness.c +++ test/Frontend/optimization-remark-with-hotness.c @@ -1,11 +1,21 @@ +// Generate instrumentation and sampling profile data. // RUN: llvm-profdata merge \ -// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ +// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ // RUN: -o %t.profdata +// RUN: llvm-profdata merge -sample \ +// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \ +// RUN: -o %t-sample.profdata +// // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ // RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ // RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \ // RUN: -Rpass-analysis=inline -Rpass-missed=inline \ // RUN: -fdiagnostics-show-hotness -verify +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ +// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ +// RUN: -fprofile-sample-use=%t-sample.profdata -Rpass=inline \ +// RUN: -Rpass-analysis=inline -Rpass-missed=inline \ +// RUN: -fdiagnostics-show-hotness -verify // The clang version of the previous test. // RUN: %clang -target x86_64-apple-macosx10.9 %s -c -emit-llvm -o /dev/null \ // RUN: -fprofile-instr-use=%t.profdata -Rpass=inline \ Index: test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext === --- /dev/null +++ test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext @@ -0,0 +1,7 @@ +foo:0:0 + 0: 0 +bar:29:29 + 6: foo:0 +main:0:0 + 0: 0 bar:0 + Index:
[PATCH] D34082: [Frontend] 'Show hotness' can be used with a sampling profile
davidxl accepted this revision. davidxl added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile
modocache updated this revision to Diff 102153. modocache added a comment. Thanks for the suggestions! I moved the sampling test close to the instrumented one, and adjusted bar and foo entry count in the hopes og getting the remarks to include hotness. No luck, however -- the test currently fails. I will look deeper into how hotness and sampling profiles work. https://reviews.llvm.org/D34082 Files: lib/Frontend/CompilerInvocation.cpp test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext test/Frontend/optimization-remark-with-hotness.c Index: test/Frontend/optimization-remark-with-hotness.c === --- test/Frontend/optimization-remark-with-hotness.c +++ test/Frontend/optimization-remark-with-hotness.c @@ -1,11 +1,21 @@ +// Generate instrumentation and sampling profile data. // RUN: llvm-profdata merge \ -// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ +// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ // RUN: -o %t.profdata +// RUN: llvm-profdata merge -sample \ +// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \ +// RUN: -o %t-sample.profdata +// // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ // RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ // RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \ // RUN: -Rpass-analysis=inline -Rpass-missed=inline \ // RUN: -fdiagnostics-show-hotness -verify +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ +// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ +// RUN: -fprofile-sample-use=%t-sample.profdata -Rpass=inline \ +// RUN: -Rpass-analysis=inline -Rpass-missed=inline \ +// RUN: -fdiagnostics-show-hotness -verify // The clang version of the previous test. // RUN: %clang -target x86_64-apple-macosx10.9 %s -c -emit-llvm -o /dev/null \ // RUN: -fprofile-instr-use=%t.profdata -Rpass=inline \ Index: test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext === --- /dev/null +++ test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext @@ -0,0 +1,6 @@ +bar:10:10 + 1: foo:10:10 +1: 10 +main:10:0 + 1: 10 + Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -884,14 +884,18 @@ Opts.DiagnosticsWithHotness = Args.hasArg(options::OPT_fdiagnostics_show_hotness); + bool UsingSampleProfile = !Opts.SampleProfileFile.empty(); + if (Opts.DiagnosticsWithHotness && - Opts.getProfileUse() == CodeGenOptions::ProfileNone) + Opts.getProfileUse() == CodeGenOptions::ProfileNone && + !UsingSampleProfile) { Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo); + } // If the user requested to use a sample profile for PGO, then the // backend will need to track source location information so the profile // can be incorporated into the IR. - if (!Opts.SampleProfileFile.empty()) + if (UsingSampleProfile) NeedLocTracking = true; // If the user requested a flag that requires source locations available in Index: test/Frontend/optimization-remark-with-hotness.c === --- test/Frontend/optimization-remark-with-hotness.c +++ test/Frontend/optimization-remark-with-hotness.c @@ -1,11 +1,21 @@ +// Generate instrumentation and sampling profile data. // RUN: llvm-profdata merge \ -// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ +// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ // RUN: -o %t.profdata +// RUN: llvm-profdata merge -sample \ +// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \ +// RUN: -o %t-sample.profdata +// // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ // RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ // RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \ // RUN: -Rpass-analysis=inline -Rpass-missed=inline \ // RUN: -fdiagnostics-show-hotness -verify +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ +// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ +// RUN: -fprofile-sample-use=%t-sample.profdata -Rpass=inline \ +// RUN: -Rpass-analysis=inline -Rpass-missed=inline \ +// RUN: -fdiagnostics-show-hotness -verify // The clang version of the previous test. // RUN: %clang -target x86_64-apple-macosx10.9 %s -c -emit-llvm -o /dev/null \ // RUN: -fprofile-instr-use=%t.profdata -Rpass=inline \ Index: test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext === --- /dev/null +++
[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile
davidxl added inline comments. Comment at: test/Frontend/optimization-remark-with-hotness.c:32 +// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness 2>&1 | FileCheck \ +// RUN: -check-prefix=PGO_ENABLED %s +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ anemet wrote: > modocache wrote: > > Ideally, I'd like for this test to be identical to the ones that use > > `-verify` above, save for using `-fprofile-sample-use`. However I couldn't > > figure out how to write `optimization-remark-with-hotness-sample.proftext` > > such that it would produce hotness info for each of the functions. I'm able > > to confirm, using real sampling from another program of mine, that remarks > > using AutoFDO data do indeed show hotness, but this test does not verify > > this in its current state. > > > > Any advice here? I wrote `optimization-remark-with-hotness-sample.proftext` > > based on the format specified in > > https://clang.llvm.org/docs/UsersManual.html#sample-profile-text-format, > > but maybe it's missing something that would allow hotness to be displayed? > I don't have any immediate ideas since I am not familiar with sample-based > profiling unfortunately. I will study this later unless you beat me to it or > David has some ideas. The hotness is based on profile summary, so you need to adjust the bar's entry count and inline instance of foo's count to be very large value to let compiler decide it is hot. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile
anemet added inline comments. Comment at: test/Frontend/optimization-remark-with-hotness.c:32 +// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness 2>&1 | FileCheck \ +// RUN: -check-prefix=PGO_ENABLED %s +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ modocache wrote: > Ideally, I'd like for this test to be identical to the ones that use > `-verify` above, save for using `-fprofile-sample-use`. However I couldn't > figure out how to write `optimization-remark-with-hotness-sample.proftext` > such that it would produce hotness info for each of the functions. I'm able > to confirm, using real sampling from another program of mine, that remarks > using AutoFDO data do indeed show hotness, but this test does not verify this > in its current state. > > Any advice here? I wrote `optimization-remark-with-hotness-sample.proftext` > based on the format specified in > https://clang.llvm.org/docs/UsersManual.html#sample-profile-text-format, but > maybe it's missing something that would allow hotness to be displayed? I don't have any immediate ideas since I am not familiar with sample-based profiling unfortunately. I will study this later unless you beat me to it or David has some ideas. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile
anemet added inline comments. Comment at: test/Frontend/optimization-remark-with-hotness.c:1-34 +// Generate instrumentation and sampling profile data. // RUN: llvm-profdata merge \ -// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ +// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ // RUN: -o %t.profdata +// RUN: llvm-profdata merge -sample \ +// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \ +// RUN: -o %t-sample.profdata Why don't you put the sampling test right under the instrumented test. Also I don't think we want a separate --check-prefix for it. It should just use the default CHECK since the entire point is that the two should be identical. In other words, please check that we don't warn on either case. https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile
modocache added inline comments. Comment at: test/Frontend/optimization-remark-with-hotness.c:32 +// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness 2>&1 | FileCheck \ +// RUN: -check-prefix=PGO_ENABLED %s +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ Ideally, I'd like for this test to be identical to the ones that use `-verify` above, save for using `-fprofile-sample-use`. However I couldn't figure out how to write `optimization-remark-with-hotness-sample.proftext` such that it would produce hotness info for each of the functions. I'm able to confirm, using real sampling from another program of mine, that remarks using AutoFDO data do indeed show hotness, but this test does not verify this in its current state. Any advice here? I wrote `optimization-remark-with-hotness-sample.proftext` based on the format specified in https://clang.llvm.org/docs/UsersManual.html#sample-profile-text-format, but maybe it's missing something that would allow hotness to be displayed? https://reviews.llvm.org/D34082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34082: [Frontend 'Show hotness' can be used with a sampling profile
modocache created this revision. Prior to this change, using `-fdiagnostics-show-hotness` with a sampling profile specified via `-fprofile-sample-use=` would result in the Clang frontend emitting a warning: "argument '-fdiagnostics-show-hotness' requires profile-guided optimization information". Of course, a sampling profile *is* profile-guided optimization information, so the warning is misleading. Furthermore, despite the warning, hotness was displayed based on the data in the sampling profile. Prevent the warning from being emitted when a sampling profile is used, and add a test that verifies this. https://reviews.llvm.org/D34082 Files: lib/Frontend/CompilerInvocation.cpp test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext test/Frontend/optimization-remark-with-hotness.c Index: test/Frontend/optimization-remark-with-hotness.c === --- test/Frontend/optimization-remark-with-hotness.c +++ test/Frontend/optimization-remark-with-hotness.c @@ -1,6 +1,11 @@ +// Generate instrumentation and sampling profile data. // RUN: llvm-profdata merge \ -// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ +// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ // RUN: -o %t.profdata +// RUN: llvm-profdata merge -sample \ +// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \ +// RUN: -o %t-sample.profdata +// // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ // RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ // RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \ @@ -22,6 +27,11 @@ // RUN: -check-prefix=HOTNESS_OFF %s // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ // RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ +// RUN: -fprofile-sample-use=%t-sample.profdata -Rpass=inline \ +// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness 2>&1 | FileCheck \ +// RUN: -check-prefix=PGO_ENABLED %s +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ +// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ // RUN: -Rpass=inline -Rpass-analysis=inline -fdiagnostics-show-hotness 2>&1 \ // RUN: | FileCheck -check-prefix=NO_PGO %s @@ -33,6 +43,7 @@ void bar(int x) { // HOTNESS_OFF: foo inlined into bar // HOTNESS_OFF-NOT: hotness: + // PGO_ENABLED-NOT: '-fdiagnostics-show-hotness' requires profile-guided optimization information // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization information // expected-remark@+2 {{foo should always be inlined (cost=always) (hotness: 30)}} // expected-remark@+1 {{foo inlined into bar (hotness: 30)}} Index: test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext === --- /dev/null +++ test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext @@ -0,0 +1,6 @@ +bar:15:0 + 1: foo:15 +1: 15 +main:100:0 + 1: 100 + Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -884,14 +884,18 @@ Opts.DiagnosticsWithHotness = Args.hasArg(options::OPT_fdiagnostics_show_hotness); + bool UsingSampleProfile = !Opts.SampleProfileFile.empty(); + if (Opts.DiagnosticsWithHotness && - Opts.getProfileUse() == CodeGenOptions::ProfileNone) + Opts.getProfileUse() == CodeGenOptions::ProfileNone && + !UsingSampleProfile) { Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo); + } // If the user requested to use a sample profile for PGO, then the // backend will need to track source location information so the profile // can be incorporated into the IR. - if (!Opts.SampleProfileFile.empty()) + if (UsingSampleProfile) NeedLocTracking = true; // If the user requested a flag that requires source locations available in Index: test/Frontend/optimization-remark-with-hotness.c === --- test/Frontend/optimization-remark-with-hotness.c +++ test/Frontend/optimization-remark-with-hotness.c @@ -1,6 +1,11 @@ +// Generate instrumentation and sampling profile data. // RUN: llvm-profdata merge \ -// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ +// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \ // RUN: -o %t.profdata +// RUN: llvm-profdata merge -sample \ +// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \ +// RUN: -o %t-sample.profdata +// // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \ // RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \ // RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \ @@ -22,6 +27,11 @@ //