[PATCH] D34868: [Driver] Add -fdiagnostics-hotness-threshold

2017-06-30 Thread Brian Gesiak via Phabricator via cfe-commits
modocache added a comment.

Awesome, thank you for all the reviews!


https://reviews.llvm.org/D34868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D34868: [Driver] Add -fdiagnostics-hotness-threshold

2017-06-30 Thread Adam Nemet via Phabricator via cfe-commits
anemet accepted this revision.
anemet added a comment.
This revision is now accepted and ready to land.

Looks great, thank you!


https://reviews.llvm.org/D34868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D34868: [Driver] Add -fdiagnostics-hotness-threshold

2017-06-30 Thread Brian Gesiak via Phabricator via cfe-commits
modocache updated this revision to Diff 104964.
modocache marked 4 inline comments as done.
modocache added a comment.

Address comments, for the most part. As for the documentation: I think this is 
OK for now, but there are definitely a few more options that I'd like to 
document, and those will require updating some of the existing docs as well 
(one example that comes to mind is '-foptimization-record-file='). If it's 
alright with you, I'd like to do this in a future diff.


https://reviews.llvm.org/D34868

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenAction.cpp
  lib/Driver/ToolChains/Clang.cpp
  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
@@ -15,24 +15,33 @@
 // 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
+// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN: -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 \
 // RUN: -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN: -fdiagnostics-show-hotness -Xclang -verify
+// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN: -Xclang -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-instrument-use-path=%t.profdata -Rpass=inline \
 // RUN: -Rpass-analysis=inline 2>&1 | FileCheck -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-instrument-use-path=%t.profdata -Rpass=inline \
-// RUN: -Rpass-analysis=inline -Rno-pass-with-hotness  2>&1 | FileCheck \
+// RUN: -Rpass-analysis=inline -Rno-pass-with-hotness 2>&1 | FileCheck \
 // 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: -Rpass=inline -Rpass-analysis=inline -fdiagnostics-show-hotness  2>&1 \
+// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
+// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness \
+// RUN: -fdiagnostics-hotness-threshold=100 2>&1 \
+// RUN: | FileCheck -allow-empty -check-prefix=THRESHOLD %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 \
+// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 2>&1 \
 // RUN: | FileCheck -check-prefix=NO_PGO %s
 
 int foo(int x, int y) __attribute__((always_inline));
@@ -43,7 +52,10 @@
 void bar(int x) {
   // HOTNESS_OFF: foo inlined into bar
   // HOTNESS_OFF-NOT: hotness:
+  // THRESHOLD-NOT: inlined
+  // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization information
+  // NO_PGO: '-fdiagnostics-hotness-threshold=' 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)}}
   sum += foo(x, x - 2);
Index: test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext
===
--- test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext
+++ test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext
@@ -1,7 +1,7 @@
 foo:0:0
  0: 0
 bar:29:29
- 6: foo:0
+ 9: foo:0
 main:0:0
  0: 0 bar:0
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -906,12 +906,18 @@
   Opts.DiagnosticsWithHotness =
   Args.hasArg(options::OPT_fdiagnostics_show_hotness);
   bool UsingSampleProfile = !Opts.SampleProfileFile.empty();
-
-  if (Opts.DiagnosticsWithHotness &&
-  Opts.getProfileUse() == CodeGenOptions::ProfileNone &&
-  !UsingSampleProfile) {
-Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo);
-  }
+  bool UsingProfile = UsingSampleProfile 

[PATCH] D34868: [Driver] Add -fdiagnostics-hotness-threshold

2017-06-30 Thread Adam Nemet via Phabricator via cfe-commits
anemet added a comment.

Great!




Comment at: docs/UsersManual.rst:330-332
This option, which defaults to off, controls whether Clang prints the
profile hotness associated with a diagnostics in the presence of
profile-guided optimization information.  This is currently supported with

While you're here, could you please update this information.  This option is 
implied by -fsave-optimization-record so it's not generally true that it's off 
by default.



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:198-203
 def warn_drv_fdiagnostics_show_hotness_requires_pgo : Warning<
   "argument '-fdiagnostics-show-hotness' requires profile-guided optimization 
information">,
   InGroup;
+def warn_drv_fdiagnostics_hotness_threshold_requires_pgo : Warning<
+  "argument '-fdiagnostics-hotness-threshold=' requires profile-guided 
optimization information">,
+  InGroup;

Can you merge these two by taking the name of the option as the parameter %0?



Comment at: include/clang/Driver/Options.td:728
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent optimization remarks from being output if they do not 
have at least this hotness value">;
 def fdiagnostics_show_option : Flag<["-"], "fdiagnostics-show-option">, 
Group,

Switch "hotness value" to "profile count"?



Comment at: lib/Frontend/CompilerInvocation.cpp:912-914
+  if (Opts.DiagnosticsWithHotness && !UsingProfile) {
 Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo);
   }

There is no {} for single statement blocks.



Comment at: test/Frontend/optimization-remark-with-hotness.c:9-20
 // 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: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN: -verify
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \

I think that one of these (or additional tests) should also cover the default 
value (i.e. omitted -fdiagnostics-hotness-threshold=).


https://reviews.llvm.org/D34868



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D34868: [Driver] Add -fdiagnostics-hotness-threshold

2017-06-30 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision.
Herald added a subscriber: fhahn.

Depends on https://reviews.llvm.org/D34867.

Add a Clang frontend option to enable optimization remark hotness
thresholds, which were added to LLVM in https://reviews.llvm.org/D34867.
This prevents diagnostics that do not meet a minimum hotness
threshold from being output. When generating optimization remarks for large
codebases with a ton of cold code paths, this option can be used
to limit the optimization remark output at a reasonable size.
Discussion of this change can be read here:
http://lists.llvm.org/pipermail/llvm-dev/2017-June/114377.html


https://reviews.llvm.org/D34868

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenAction.cpp
  lib/Driver/ToolChains/Clang.cpp
  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
@@ -10,29 +10,39 @@
 // 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: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN: -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
+// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN: -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 \
 // RUN: -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN: -fdiagnostics-show-hotness -Xclang -verify
+// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN: -Xclang -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-instrument-use-path=%t.profdata -Rpass=inline \
 // RUN: -Rpass-analysis=inline 2>&1 | FileCheck -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-instrument-use-path=%t.profdata -Rpass=inline \
-// RUN: -Rpass-analysis=inline -Rno-pass-with-hotness  2>&1 | FileCheck \
+// RUN: -Rpass-analysis=inline -Rno-pass-with-hotness 2>&1 | FileCheck \
 // 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: -Rpass=inline -Rpass-analysis=inline -fdiagnostics-show-hotness  2>&1 \
+// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
+// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness \
+// RUN: -fdiagnostics-hotness-threshold=100 2>&1 \
+// RUN: | FileCheck -allow-empty -check-prefix=THRESHOLD %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 \
+// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 2>&1 \
 // RUN: | FileCheck -check-prefix=NO_PGO %s
 
 int foo(int x, int y) __attribute__((always_inline));
@@ -43,7 +53,10 @@
 void bar(int x) {
   // HOTNESS_OFF: foo inlined into bar
   // HOTNESS_OFF-NOT: hotness:
+  // THRESHOLD-NOT: inlined
+  // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization information
+  // NO_PGO: '-fdiagnostics-hotness-threshold=' 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)}}
   sum += foo(x, x - 2);
Index: test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext
===
--- test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext
+++ test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext
@@ -1,7 +1,7 @@
 foo:0:0
  0: 0
 bar:29:29
- 6: foo:0
+ 9: foo:0
 main:0:0
  0: 0 bar:0
 
Index: lib/Frontend/CompilerInvocation.cpp