[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-26 Thread Vlad Vereschaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG155c49e0878d: [Driver] Print process statistics report on 
CC_PRINT_PROC_STAT env variable. (authored by vvereschaka).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cc-print-proc-stat.c
  clang/test/Driver/lit.local.cfg
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -258,6 +258,11 @@
   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
 TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Handle CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE.
+  TheDriver.CCPrintProcessStats = !!::getenv("CC_PRINT_PROC_STAT");
+  if (TheDriver.CCPrintProcessStats)
+TheDriver.CCPrintStatReportFilename = ::getenv("CC_PRINT_PROC_STAT_FILE");
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -11,7 +11,7 @@
  'IPHONEOS_DEPLOYMENT_TARGET',
  'SDKROOT', 'CCC_OVERRIDE_OPTIONS',
  'CC_PRINT_OPTIONS', 'CC_PRINT_HEADERS',
- 'CC_LOG_DIAGNOSTICS']
+ 'CC_LOG_DIAGNOSTICS', 'CC_PRINT_PROC_STAT']
 
 for name in driver_overwrite_env_vars:
   if name in config.environment:
Index: clang/test/Driver/cc-print-proc-stat.c
===
--- /dev/null
+++ clang/test/Driver/cc-print-proc-stat.c
@@ -0,0 +1,9 @@
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: CC_PRINT_PROC_STAT_FILE=%t.csv \
+// RUN: %clang -no-canonical-prefixes -S -o %t.s %s
+// RUN: FileCheck --check-prefix=CHECK-CSV %s < %t.csv
+// CHECK-CSV: clang{{.*}},"{{.*}}.s",{{[0-9]+}},{{[0-9]+}},{{[0-9]+}}
+
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: %clang -c -fintegrated-as %s | FileCheck %s
+// CHECK: clang{{.*}}: output={{.*}}.o, total={{[0-9.]+}} ms, user={{[0-9.]+}} ms, mem={{[0-9]+}} Kb
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -137,8 +137,9 @@
   ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
   DriverTitle(Title), CCPrintOptionsFilename(nullptr),
   CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
-  CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
-  CCLogDiagnostics(false), CCGenDiagnostics(false),
+  CCPrintStatReportFilename(nullptr), CCCPrintBindings(false),
+  CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
+  CCGenDiagnostics(false), CCPrintProcessStats(false),
   TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
   CheckInputsExist(true), GenReproducer(false),
   SuppressMissingInputWarning(false) {
@@ -1096,6 +1097,15 @@
   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
options::OPT_fno_crash_diagnostics,
!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
+
+  // Process -fproc-stat-report options.
+  if (const Arg *A = Args.getLastArg(options::OPT_fproc_stat_report_EQ)) {
+CCPrintProcessStats = true;
+CCPrintStatReportFilename = A->getValue();
+  }
+  if (Args.hasArg(options::OPT_fproc_stat_report))
+CCPrintProcessStats = true;
+
   // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {
@@ -4009,62 +4019,58 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
-  StringRef StatReportFile;
-  bool PrintProcessStat = false;
-  if (const Arg *A = C.getArgs().getLastArg(options::OPT_fproc_stat_report_EQ))
-StatReportFile = A->getValue();
-  if (C.getArgs().hasArg(options::OPT_fproc_stat_report))
-PrintProcessStat = true;
-
   // If we have more than one job, then disable integrated-cc1 for now. Do this
   // also when we need to report process execution statistics.
-  if (C.getJobs().size() > 1 || !StatReportFile.empty() || PrintProcessStat)
+  if (C.getJobs().size() > 1 || CCPrintProcessStats)
 for (auto  : C.getJobs())
   J.InProcess = false;
 
-  if (!StatReportFile.empty() || PrintProcessStat) {
+  if (CCPrintProcessStats) {
 C.setPostCallback([=](const Command , int Res) {
   Optional ProcStat =
   Cmd.getProcessStatistics();
   if (!ProcStat)
 

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-26 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka updated this revision to Diff 326729.
vvereschaka added a comment.

Updated diff:

- updated doc parts

@aganea thanks a lot for your suggestions for doc improvements. I have updated 
it accordingly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cc-print-proc-stat.c
  clang/test/Driver/lit.local.cfg
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -258,6 +258,11 @@
   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
 TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Handle CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE.
+  TheDriver.CCPrintProcessStats = !!::getenv("CC_PRINT_PROC_STAT");
+  if (TheDriver.CCPrintProcessStats)
+TheDriver.CCPrintStatReportFilename = ::getenv("CC_PRINT_PROC_STAT_FILE");
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -11,7 +11,7 @@
  'IPHONEOS_DEPLOYMENT_TARGET',
  'SDKROOT', 'CCC_OVERRIDE_OPTIONS',
  'CC_PRINT_OPTIONS', 'CC_PRINT_HEADERS',
- 'CC_LOG_DIAGNOSTICS']
+ 'CC_LOG_DIAGNOSTICS', 'CC_PRINT_PROC_STAT']
 
 for name in driver_overwrite_env_vars:
   if name in config.environment:
Index: clang/test/Driver/cc-print-proc-stat.c
===
--- /dev/null
+++ clang/test/Driver/cc-print-proc-stat.c
@@ -0,0 +1,9 @@
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: CC_PRINT_PROC_STAT_FILE=%t.csv \
+// RUN: %clang -no-canonical-prefixes -S -o %t.s %s
+// RUN: FileCheck --check-prefix=CHECK-CSV %s < %t.csv
+// CHECK-CSV: clang{{.*}},"{{.*}}.s",{{[0-9]+}},{{[0-9]+}},{{[0-9]+}}
+
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: %clang -c -fintegrated-as %s | FileCheck %s
+// CHECK: clang{{.*}}: output={{.*}}.o, total={{[0-9.]+}} ms, user={{[0-9.]+}} ms, mem={{[0-9]+}} Kb
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -137,8 +137,9 @@
   ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
   DriverTitle(Title), CCPrintOptionsFilename(nullptr),
   CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
-  CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
-  CCLogDiagnostics(false), CCGenDiagnostics(false),
+  CCPrintStatReportFilename(nullptr), CCCPrintBindings(false),
+  CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
+  CCGenDiagnostics(false), CCPrintProcessStats(false),
   TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
   CheckInputsExist(true), GenReproducer(false),
   SuppressMissingInputWarning(false) {
@@ -1096,6 +1097,15 @@
   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
options::OPT_fno_crash_diagnostics,
!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
+
+  // Process -fproc-stat-report options.
+  if (const Arg *A = Args.getLastArg(options::OPT_fproc_stat_report_EQ)) {
+CCPrintProcessStats = true;
+CCPrintStatReportFilename = A->getValue();
+  }
+  if (Args.hasArg(options::OPT_fproc_stat_report))
+CCPrintProcessStats = true;
+
   // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {
@@ -4009,62 +4019,58 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
-  StringRef StatReportFile;
-  bool PrintProcessStat = false;
-  if (const Arg *A = C.getArgs().getLastArg(options::OPT_fproc_stat_report_EQ))
-StatReportFile = A->getValue();
-  if (C.getArgs().hasArg(options::OPT_fproc_stat_report))
-PrintProcessStat = true;
-
   // If we have more than one job, then disable integrated-cc1 for now. Do this
   // also when we need to report process execution statistics.
-  if (C.getJobs().size() > 1 || !StatReportFile.empty() || PrintProcessStat)
+  if (C.getJobs().size() > 1 || CCPrintProcessStats)
 for (auto  : C.getJobs())
   J.InProcess = false;
 
-  if (!StatReportFile.empty() || PrintProcessStat) {
+  if (CCPrintProcessStats) {
 C.setPostCallback([=](const Command , int Res) {
   Optional ProcStat =
   Cmd.getProcessStatistics();
   if (!ProcStat)
 

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-26 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea accepted this revision.
aganea added a comment.
This revision is now accepted and ready to land.

LGTM, just a few minor things:




Comment at: clang/docs/UsersManual.rst:798
+  Setting ``CC_PRINT_PROC_STAT`` to ``1`` enables the feature, the report goes 
to
+  the stdout in the human readable format.
+  Setting ``CC_PRINT_PROC_STAT_FILE`` to a fully qualified file path makes it 
report

"enables the feature, the report goes to ~~the~~ stdout in ~~the~~ human 
readable format."



Comment at: clang/docs/UsersManual.rst:800
+  Setting ``CC_PRINT_PROC_STAT_FILE`` to a fully qualified file path makes it 
report
+  the process statistics to the given file in the CSV format. Specifying a 
relative
+  path will likely lead to multiple files with the same name created in 
different

"~~the~~ process statistics to the given file in the CSV format"



Comment at: clang/docs/UsersManual.rst:804
+
+  These environment variables are handy when you need to request the statistics
+  report without changing your build scripts or alter the existing set of 
compiler

Would you be able to merge this paragraph with the following one please? It 
could be something like:

"These environment variables are handy when you need to request the statistics 
report without changing your build scripts or alter the existing set of 
compiler options. Note that `-fproc-stat-report` take precedence over 
`CC_PRINT_PROC_STAT` and `CC_PRINT_PROC_STAT_FILE`."


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-25 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka added inline comments.



Comment at: clang/docs/UsersManual.rst:799
+  setting the ``CC_PRINT_PROC_STAT`` and ``CC_PRINT_PROC_STAT_FILE`` 
environment
+  variables. Use ``CC_PRINT_PROC_STAT_FILE`` to provide a file name to store
+  the statistics.

aganea wrote:
> Do you think it would be possible to rephrase a bit to avoid the repetition 
> of `CC_PRINT_PROC_STAT_FILE`? Perhaps also explicitate that 
> `CC_PRINT_PROC_STAT` is for "enabling the feature and logging to stdout"; 
> while `CC_PRINT_PROC_STAT_FILE` only "redirects the log to a file".
Yes, sure. Here is updated and extended version of the doc part.



Comment at: clang/lib/Driver/Driver.cpp:4039
+
+  if (CCPrintStatReportFilename == nullptr) {
 using namespace llvm;

aganea wrote:
> Suggestion: I find `if (!CCPrintStatReportFilename)` more intutive, more 
> compact and easier to read, but that's my personal preference. There are 
> arguments both ways probably, up to you.
I have updated it in according of you suggestion. It is ok for me too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-25 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka updated this revision to Diff 326477.
vvereschaka added a comment.

Updated diff:

- rewording the doc part
- removed `ClaimAllArgs` for `OPT_fproc_stat_report`
- updated `if` statement for `CCPrintStatReportFilename`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cc-print-proc-stat.c
  clang/test/Driver/lit.local.cfg
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -258,6 +258,11 @@
   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
 TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Handle CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE.
+  TheDriver.CCPrintProcessStats = !!::getenv("CC_PRINT_PROC_STAT");
+  if (TheDriver.CCPrintProcessStats)
+TheDriver.CCPrintStatReportFilename = ::getenv("CC_PRINT_PROC_STAT_FILE");
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -11,7 +11,7 @@
  'IPHONEOS_DEPLOYMENT_TARGET',
  'SDKROOT', 'CCC_OVERRIDE_OPTIONS',
  'CC_PRINT_OPTIONS', 'CC_PRINT_HEADERS',
- 'CC_LOG_DIAGNOSTICS']
+ 'CC_LOG_DIAGNOSTICS', 'CC_PRINT_PROC_STAT']
 
 for name in driver_overwrite_env_vars:
   if name in config.environment:
Index: clang/test/Driver/cc-print-proc-stat.c
===
--- /dev/null
+++ clang/test/Driver/cc-print-proc-stat.c
@@ -0,0 +1,9 @@
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: CC_PRINT_PROC_STAT_FILE=%t.csv \
+// RUN: %clang -no-canonical-prefixes -S -o %t.s %s
+// RUN: FileCheck --check-prefix=CHECK-CSV %s < %t.csv
+// CHECK-CSV: clang{{.*}},"{{.*}}.s",{{[0-9]+}},{{[0-9]+}},{{[0-9]+}}
+
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: %clang -c -fintegrated-as %s | FileCheck %s
+// CHECK: clang{{.*}}: output={{.*}}.o, total={{[0-9.]+}} ms, user={{[0-9.]+}} ms, mem={{[0-9]+}} Kb
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -137,8 +137,9 @@
   ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
   DriverTitle(Title), CCPrintOptionsFilename(nullptr),
   CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
-  CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
-  CCLogDiagnostics(false), CCGenDiagnostics(false),
+  CCPrintStatReportFilename(nullptr), CCCPrintBindings(false),
+  CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
+  CCGenDiagnostics(false), CCPrintProcessStats(false),
   TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
   CheckInputsExist(true), GenReproducer(false),
   SuppressMissingInputWarning(false) {
@@ -1096,6 +1097,15 @@
   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
options::OPT_fno_crash_diagnostics,
!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
+
+  // Process -fproc-stat-report options.
+  if (const Arg *A = Args.getLastArg(options::OPT_fproc_stat_report_EQ)) {
+CCPrintProcessStats = true;
+CCPrintStatReportFilename = A->getValue();
+  }
+  if (Args.hasArg(options::OPT_fproc_stat_report))
+CCPrintProcessStats = true;
+
   // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {
@@ -4004,62 +4014,58 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
-  StringRef StatReportFile;
-  bool PrintProcessStat = false;
-  if (const Arg *A = C.getArgs().getLastArg(options::OPT_fproc_stat_report_EQ))
-StatReportFile = A->getValue();
-  if (C.getArgs().hasArg(options::OPT_fproc_stat_report))
-PrintProcessStat = true;
-
   // If we have more than one job, then disable integrated-cc1 for now. Do this
   // also when we need to report process execution statistics.
-  if (C.getJobs().size() > 1 || !StatReportFile.empty() || PrintProcessStat)
+  if (C.getJobs().size() > 1 || CCPrintProcessStats)
 for (auto  : C.getJobs())
   J.InProcess = false;
 
-  if (!StatReportFile.empty() || PrintProcessStat) {
+  if (CCPrintProcessStats) {
 C.setPostCallback([=](const Command , int Res) {
   Optional ProcStat =
   Cmd.getProcessStatistics();
   if 

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-24 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

Thanks for the update @vvereschaka ! Just a few minor things and it should be 
good to go.
Also please use `git diff -U9` next time to add context to the patch.




Comment at: clang/docs/UsersManual.rst:783
   It is possible to specify this option without any value. In this case 
statistics
   is printed on standard output in human readable format:
   

I think this should read //"In this case statistics **are** printed.."// Would 
you mind changing this as well?



Comment at: clang/docs/UsersManual.rst:799
+  setting the ``CC_PRINT_PROC_STAT`` and ``CC_PRINT_PROC_STAT_FILE`` 
environment
+  variables. Use ``CC_PRINT_PROC_STAT_FILE`` to provide a file name to store
+  the statistics.

Do you think it would be possible to rephrase a bit to avoid the repetition of 
`CC_PRINT_PROC_STAT_FILE`? Perhaps also explicitate that `CC_PRINT_PROC_STAT` 
is for "enabling the feature and logging to stdout"; while 
`CC_PRINT_PROC_STAT_FILE` only "redirects the log to a file".



Comment at: clang/lib/Driver/Driver.cpp:1108
+CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+

Remove this too, `hasArg` claims all arguments.



Comment at: clang/lib/Driver/Driver.cpp:4039
+
+  if (CCPrintStatReportFilename == nullptr) {
 using namespace llvm;

Suggestion: I find `if (!CCPrintStatReportFilename)` more intutive, more 
compact and easier to read, but that's my personal preference. There are 
arguments both ways probably, up to you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-24 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4050
<< ", mem=" << ProcStat->PeakMemory << " Kb\n";
-  }
-  if (!StatReportFile.empty()) {
+  } else {
 // CSV format.

vvereschaka wrote:
> aganea wrote:
> > The previous behavior was printing both the output and the CSV file when 
> > specifying `-fproc-stat-report -fproc-stat-report=file.csv`, now it would 
> > only emit the CSV, is that intended? @sepavloff what do you think?
> >now it would only emit the CSV, is that intended?
> 
> yes, I thought in that way. I dоn't think that usage of both options at the 
> same time was specially designed. I don't see a reasonable usage case for 
> that for now actually, but I could be wrong. @sepavloff ?
> > now it would only emit the CSV, is that intended?

> yes, I thought in that way. I dоn't think that usage of both options at the 
> same time was specially designed. I don't see a reasonable usage case for 
> that for now actually, but I could be wrong. @sepavloff ?

The previous behavior (printing both the output and the CSV file) was not 
intentional, it was the implementation peculiarity. I don't see real use case 
for both outputs. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-23 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4050
<< ", mem=" << ProcStat->PeakMemory << " Kb\n";
-  }
-  if (!StatReportFile.empty()) {
+  } else {
 // CSV format.

aganea wrote:
> The previous behavior was printing both the output and the CSV file when 
> specifying `-fproc-stat-report -fproc-stat-report=file.csv`, now it would 
> only emit the CSV, is that intended? @sepavloff what do you think?
>now it would only emit the CSV, is that intended?

yes, I thought in that way. I dоn't think that usage of both options at the 
same time was specially designed. I don't see a reasonable usage case for that 
for now actually, but I could be wrong. @sepavloff ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-23 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka updated this revision to Diff 325930.
vvereschaka added a comment.

Thank you @aganea

Updated diff:

- removed `ClaimAllArgs` for `OPT_fproc_stat_report_EQ` (used in `getLastArg`);
- used `const char*` instead of `std::string` for `CCPrintStatReportFilename`;
- removed unnecessary comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cc-print-proc-stat.c
  clang/test/Driver/lit.local.cfg
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -258,6 +258,11 @@
   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
 TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Handle CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE.
+  TheDriver.CCPrintProcessStats = !!::getenv("CC_PRINT_PROC_STAT");
+  if (TheDriver.CCPrintProcessStats)
+TheDriver.CCPrintStatReportFilename = ::getenv("CC_PRINT_PROC_STAT_FILE");
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -11,7 +11,7 @@
  'IPHONEOS_DEPLOYMENT_TARGET',
  'SDKROOT', 'CCC_OVERRIDE_OPTIONS',
  'CC_PRINT_OPTIONS', 'CC_PRINT_HEADERS',
- 'CC_LOG_DIAGNOSTICS']
+ 'CC_LOG_DIAGNOSTICS', 'CC_PRINT_PROC_STAT']
 
 for name in driver_overwrite_env_vars:
   if name in config.environment:
Index: clang/test/Driver/cc-print-proc-stat.c
===
--- /dev/null
+++ clang/test/Driver/cc-print-proc-stat.c
@@ -0,0 +1,9 @@
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: CC_PRINT_PROC_STAT_FILE=%t.csv \
+// RUN: %clang -no-canonical-prefixes -S -o %t.s %s
+// RUN: FileCheck --check-prefix=CHECK-CSV %s < %t.csv
+// CHECK-CSV: clang{{.*}},"{{.*}}.s",{{[0-9]+}},{{[0-9]+}},{{[0-9]+}}
+
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: %clang -c -fintegrated-as %s | FileCheck %s
+// CHECK: clang{{.*}}: output={{.*}}.o, total={{[0-9.]+}} ms, user={{[0-9.]+}} ms, mem={{[0-9]+}} Kb
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -137,8 +137,9 @@
   ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
   DriverTitle(Title), CCPrintOptionsFilename(nullptr),
   CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
-  CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
-  CCLogDiagnostics(false), CCGenDiagnostics(false),
+  CCPrintStatReportFilename(nullptr), CCCPrintBindings(false),
+  CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
+  CCGenDiagnostics(false), CCPrintProcessStats(false),
   TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
   CheckInputsExist(true), GenReproducer(false),
   SuppressMissingInputWarning(false) {
@@ -1096,6 +1097,16 @@
   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
options::OPT_fno_crash_diagnostics,
!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
+
+  // Process -fproc-stat-report options.
+  if (const Arg *A = Args.getLastArg(options::OPT_fproc_stat_report_EQ)) {
+CCPrintProcessStats = true;
+CCPrintStatReportFilename = A->getValue();
+  }
+  if (Args.hasArg(options::OPT_fproc_stat_report))
+CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+
   // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {
@@ -4004,62 +4015,58 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
-  StringRef StatReportFile;
-  bool PrintProcessStat = false;
-  if (const Arg *A = C.getArgs().getLastArg(options::OPT_fproc_stat_report_EQ))
-StatReportFile = A->getValue();
-  if (C.getArgs().hasArg(options::OPT_fproc_stat_report))
-PrintProcessStat = true;
-
   // If we have more than one job, then disable integrated-cc1 for now. Do this
   // also when we need to report process execution statistics.
-  if (C.getJobs().size() > 1 || !StatReportFile.empty() || PrintProcessStat)
+  if (C.getJobs().size() > 1 || CCPrintProcessStats)
 for (auto  : C.getJobs())
   J.InProcess = false;
 
-  if (!StatReportFile.empty() || PrintProcessStat) {
+  if (CCPrintProcessStats) {
 

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-22 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1107
+CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report_EQ);

Please remove, `getLastArg` already claims all arguments for this option.



Comment at: clang/lib/Driver/Driver.cpp:4031
+
+  // Use FinalOutput variable to get the output file name.
+  const char *LinkingOutput = nullptr;

I think the code below is safe-explanatory, I don't think you need this 
comment, it doesn't add much value.



Comment at: clang/lib/Driver/Driver.cpp:4050
<< ", mem=" << ProcStat->PeakMemory << " Kb\n";
-  }
-  if (!StatReportFile.empty()) {
+  } else {
 // CSV format.

The previous behavior was printing both the output and the CSV file when 
specifying `-fproc-stat-report -fproc-stat-report=file.csv`, now it would only 
emit the CSV, is that intended? @sepavloff what do you think?



Comment at: clang/test/Driver/cc-print-proc-stat.c:1
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: CC_PRINT_PROC_STAT_FILE=%t.csv \

Could you please expand a bit the test to entirely cover all the codepaths 
above?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-22 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka marked an inline comment as done.
vvereschaka added inline comments.



Comment at: clang/include/clang/Driver/Driver.h:210
 
+  /// Set CC_PRINT_PROC_STAT mode, which causes the frontend to dump
+  /// performance report to CC_PRINT_PROC_STAT_FILE or to stdout.

sepavloff wrote:
> Strictly speaking it is driver that dumps performance report, not frontend.
Thanks, I have updated the comment accordingly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-22 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka updated this revision to Diff 325579.
vvereschaka added a comment.

Fixed comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cc-print-proc-stat.c
  clang/test/Driver/lit.local.cfg
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -258,6 +258,11 @@
   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
 TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Handle CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE.
+  TheDriver.CCPrintProcessStats = !!::getenv("CC_PRINT_PROC_STAT");
+  if (TheDriver.CCPrintProcessStats)
+TheDriver.CCPrintStatReportFilename = ::getenv("CC_PRINT_PROC_STAT_FILE");
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -11,7 +11,7 @@
  'IPHONEOS_DEPLOYMENT_TARGET',
  'SDKROOT', 'CCC_OVERRIDE_OPTIONS',
  'CC_PRINT_OPTIONS', 'CC_PRINT_HEADERS',
- 'CC_LOG_DIAGNOSTICS']
+ 'CC_LOG_DIAGNOSTICS', 'CC_PRINT_PROC_STAT']
 
 for name in driver_overwrite_env_vars:
   if name in config.environment:
Index: clang/test/Driver/cc-print-proc-stat.c
===
--- /dev/null
+++ clang/test/Driver/cc-print-proc-stat.c
@@ -0,0 +1,6 @@
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: CC_PRINT_PROC_STAT_FILE=%t.csv \
+// RUN: %clang -no-canonical-prefixes -S -o %t.s %s
+// RUN: FileCheck --check-prefix=CSV %s < %t.csv
+
+// CSV: clang{{.*}},"{{.*}}.s",{{[0-9]+}},{{[0-9]+}},{{[0-9]+}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -139,9 +139,9 @@
   CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
   CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
   CCLogDiagnostics(false), CCGenDiagnostics(false),
-  TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
-  CheckInputsExist(true), GenReproducer(false),
-  SuppressMissingInputWarning(false) {
+  CCPrintProcessStats(false), TargetTriple(TargetTriple),
+  CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
+  GenReproducer(false), SuppressMissingInputWarning(false) {
   // Provide a sane fallback if no VFS is specified.
   if (!this->VFS)
 this->VFS = llvm::vfs::getRealFileSystem();
@@ -1096,6 +1096,17 @@
   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
options::OPT_fno_crash_diagnostics,
!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
+
+  // Process -fproc-stat-report options.
+  if (const Arg *A = Args.getLastArg(options::OPT_fproc_stat_report_EQ)) {
+CCPrintProcessStats = true;
+CCPrintStatReportFilename = A->getValue();
+  }
+  if (Args.hasArg(options::OPT_fproc_stat_report))
+CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report_EQ);
+
   // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {
@@ -4004,62 +4015,59 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
-  StringRef StatReportFile;
-  bool PrintProcessStat = false;
-  if (const Arg *A = C.getArgs().getLastArg(options::OPT_fproc_stat_report_EQ))
-StatReportFile = A->getValue();
-  if (C.getArgs().hasArg(options::OPT_fproc_stat_report))
-PrintProcessStat = true;
-
   // If we have more than one job, then disable integrated-cc1 for now. Do this
   // also when we need to report process execution statistics.
-  if (C.getJobs().size() > 1 || !StatReportFile.empty() || PrintProcessStat)
+  if (C.getJobs().size() > 1 || CCPrintProcessStats)
 for (auto  : C.getJobs())
   J.InProcess = false;
 
-  if (!StatReportFile.empty() || PrintProcessStat) {
+  if (CCPrintProcessStats) {
 C.setPostCallback([=](const Command , int Res) {
   Optional ProcStat =
   Cmd.getProcessStatistics();
   if (!ProcStat)
 return;
-  if (PrintProcessStat) {
+
+  // Use FinalOutput variable to get the output file name.
+  const char *LinkingOutput = nullptr;
+  if (FinalOutput)
+LinkingOutput = FinalOutput->getValue();
+   

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-20 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/include/clang/Driver/Driver.h:210
 
+  /// Set CC_PRINT_PROC_STAT mode, which causes the frontend to dump
+  /// performance report to CC_PRINT_PROC_STAT_FILE or to stdout.

Strictly speaking it is driver that dumps performance report, not frontend.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-19 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka updated this revision to Diff 325101.
vvereschaka added a comment.

Updated diff in according of the Lint/clang-format recommendations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cc-print-proc-stat.c
  clang/test/Driver/lit.local.cfg
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -258,6 +258,11 @@
   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
 TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Handle CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE.
+  TheDriver.CCPrintProcessStats = !!::getenv("CC_PRINT_PROC_STAT");
+  if (TheDriver.CCPrintProcessStats)
+TheDriver.CCPrintStatReportFilename = ::getenv("CC_PRINT_PROC_STAT_FILE");
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -11,7 +11,7 @@
  'IPHONEOS_DEPLOYMENT_TARGET',
  'SDKROOT', 'CCC_OVERRIDE_OPTIONS',
  'CC_PRINT_OPTIONS', 'CC_PRINT_HEADERS',
- 'CC_LOG_DIAGNOSTICS']
+ 'CC_LOG_DIAGNOSTICS', 'CC_PRINT_PROC_STAT']
 
 for name in driver_overwrite_env_vars:
   if name in config.environment:
Index: clang/test/Driver/cc-print-proc-stat.c
===
--- /dev/null
+++ clang/test/Driver/cc-print-proc-stat.c
@@ -0,0 +1,6 @@
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: CC_PRINT_PROC_STAT_FILE=%t.csv \
+// RUN: %clang -no-canonical-prefixes -S -o %t.s %s
+// RUN: FileCheck --check-prefix=CSV %s < %t.csv
+
+// CSV: clang{{.*}},"{{.*}}.s",{{[0-9]+}},{{[0-9]+}},{{[0-9]+}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -139,9 +139,9 @@
   CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
   CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
   CCLogDiagnostics(false), CCGenDiagnostics(false),
-  TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
-  CheckInputsExist(true), GenReproducer(false),
-  SuppressMissingInputWarning(false) {
+  CCPrintProcessStats(false), TargetTriple(TargetTriple),
+  CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
+  GenReproducer(false), SuppressMissingInputWarning(false) {
   // Provide a sane fallback if no VFS is specified.
   if (!this->VFS)
 this->VFS = llvm::vfs::getRealFileSystem();
@@ -1096,6 +1096,17 @@
   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
options::OPT_fno_crash_diagnostics,
!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
+
+  // Process -fproc-stat-report options.
+  if (const Arg *A = Args.getLastArg(options::OPT_fproc_stat_report_EQ)) {
+CCPrintProcessStats = true;
+CCPrintStatReportFilename = A->getValue();
+  }
+  if (Args.hasArg(options::OPT_fproc_stat_report))
+CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report_EQ);
+
   // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {
@@ -4004,62 +4015,59 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
-  StringRef StatReportFile;
-  bool PrintProcessStat = false;
-  if (const Arg *A = C.getArgs().getLastArg(options::OPT_fproc_stat_report_EQ))
-StatReportFile = A->getValue();
-  if (C.getArgs().hasArg(options::OPT_fproc_stat_report))
-PrintProcessStat = true;
-
   // If we have more than one job, then disable integrated-cc1 for now. Do this
   // also when we need to report process execution statistics.
-  if (C.getJobs().size() > 1 || !StatReportFile.empty() || PrintProcessStat)
+  if (C.getJobs().size() > 1 || CCPrintProcessStats)
 for (auto  : C.getJobs())
   J.InProcess = false;
 
-  if (!StatReportFile.empty() || PrintProcessStat) {
+  if (CCPrintProcessStats) {
 C.setPostCallback([=](const Command , int Res) {
   Optional ProcStat =
   Cmd.getProcessStatistics();
   if (!ProcStat)
 return;
-  if (PrintProcessStat) {
+
+  // Use FinalOutput variable to get the output file name.
+  const char *LinkingOutput = nullptr;
+  if (FinalOutput)

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-19 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka added a comment.

@sepavloff would you add more persons in additional to the reviewers?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094

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


[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

2021-02-19 Thread Vlad Vereschaka via Phabricator via cfe-commits
vvereschaka created this revision.
vvereschaka added a reviewer: sepavloff.
vvereschaka added a project: clang.
vvereschaka requested review of this revision.
Herald added a subscriber: cfe-commits.

Added supporting CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE environment 
variables to trigger clang driver reporting the process statistics into 
specified file (alternate for -fproc-stat-report option).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97094

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cc-print-proc-stat.c
  clang/test/Driver/lit.local.cfg
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -258,6 +258,11 @@
   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
 TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Handle CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE.
+  TheDriver.CCPrintProcessStats = !!::getenv("CC_PRINT_PROC_STAT");
+  if (TheDriver.CCPrintProcessStats)
+TheDriver.CCPrintStatReportFilename = ::getenv("CC_PRINT_PROC_STAT_FILE");
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -11,7 +11,7 @@
  'IPHONEOS_DEPLOYMENT_TARGET',
  'SDKROOT', 'CCC_OVERRIDE_OPTIONS',
  'CC_PRINT_OPTIONS', 'CC_PRINT_HEADERS',
- 'CC_LOG_DIAGNOSTICS']
+ 'CC_LOG_DIAGNOSTICS', 'CC_PRINT_PROC_STAT']
 
 for name in driver_overwrite_env_vars:
   if name in config.environment:
Index: clang/test/Driver/cc-print-proc-stat.c
===
--- /dev/null
+++ clang/test/Driver/cc-print-proc-stat.c
@@ -0,0 +1,6 @@
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN: CC_PRINT_PROC_STAT_FILE=%t.csv \
+// RUN: %clang -no-canonical-prefixes -S -o %t.s %s
+// RUN: FileCheck --check-prefix=CSV %s < %t.csv
+
+// CSV: clang{{.*}},"{{.*}}.s",{{[0-9]+}},{{[0-9]+}},{{[0-9]+}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -138,7 +138,7 @@
   DriverTitle(Title), CCPrintOptionsFilename(nullptr),
   CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
   CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
-  CCLogDiagnostics(false), CCGenDiagnostics(false),
+  CCLogDiagnostics(false), CCGenDiagnostics(false), CCPrintProcessStats(false),
   TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
   CheckInputsExist(true), GenReproducer(false),
   SuppressMissingInputWarning(false) {
@@ -1096,6 +1096,17 @@
   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
options::OPT_fno_crash_diagnostics,
!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
+
+  // Process -fproc-stat-report options.
+  if (const Arg *A = Args.getLastArg(options::OPT_fproc_stat_report_EQ)) {
+CCPrintProcessStats = true;
+CCPrintStatReportFilename = A->getValue();
+  }
+  if (Args.hasArg(options::OPT_fproc_stat_report))
+CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report_EQ);
+
   // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {
@@ -4004,62 +4015,58 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
-  StringRef StatReportFile;
-  bool PrintProcessStat = false;
-  if (const Arg *A = C.getArgs().getLastArg(options::OPT_fproc_stat_report_EQ))
-StatReportFile = A->getValue();
-  if (C.getArgs().hasArg(options::OPT_fproc_stat_report))
-PrintProcessStat = true;
-
   // If we have more than one job, then disable integrated-cc1 for now. Do this
   // also when we need to report process execution statistics.
-  if (C.getJobs().size() > 1 || !StatReportFile.empty() || PrintProcessStat)
+  if (C.getJobs().size() > 1 || CCPrintProcessStats)
 for (auto  : C.getJobs())
   J.InProcess = false;
 
-  if (!StatReportFile.empty() || PrintProcessStat) {
+  if (CCPrintProcessStats) {
 C.setPostCallback([=](const Command , int Res) {
   Optional ProcStat =
   Cmd.getProcessStatistics();
   if (!ProcStat)
 return;
-  if (PrintProcessStat) {
+
+  // Use FinalOutput variable to get the output file name.
+  const char *LinkingOutput = nullptr;
+