[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D47196#1202805, @efriedma wrote:

> > :start: means the timer was started
>
> In some cases, the ChildTime is already non-zero at the "start" point; what 
> does that mean?


It means there was recursive call and this child time should be removed from 
the time of the current func (parent) otherwise it could be calculated twice.

>> ActOnFunctionDeclarator:start:._exception:
> 
> That's weird... DeclarationName::print should generally return something 
> ASCII; do you know what kind of declaration it is?

There are a lot of such names (see the attach as example.)F6964637: b.log 

BTW, I use getNameAsString() (see getFirstName). I tried getIdentifier () 
->getName() but it was broken rather often.


https://reviews.llvm.org/D47196



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D47196#1190377, @efriedma wrote:

> I mean, which of the callers of startFrontendTimer() is calling it with a 
> pointer to std::declval()?


F6951300: declval.log 

The attachment keeps debug output for

../bin/clang -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mrelax-all 
-disable-free -disable-llvm-verifier -discard-value-names -main-file-name 
TransGCCalls.cpp -mrelocation-model pic -pic-level 2 -mthread-model posix 
-mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases 
-munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info 
-debugger-tuning=gdb -v -v -coverage-notes-file 
/home/atischenko/workspaces/bootstrap/Release-patch/tools/clang/lib/ARCMigrate/CMakeFiles/clangARCMigrate.dir/TransGCCalls.cpp.gcno
 -resource-dir 
/home/atischenko/workspaces/time-report-recursive-4/RelWithDebug/lib/clang/8.0.0
 -D GTEST_HAS_RTTI=0 -D _DEBUG -D _GNU_SOURCE -D __STDC_CONSTANT_MACROS -D 
__STDC_FORMAT_MACROS -D __STDC_LIMIT_MACROS -I 
/home/atischenko/workspaces/bootstrap/Release-patch/tools/clang/lib/ARCMigrate 
-I /home/atischenko/workspaces/bootstrap/llvm/tools/clang/lib/ARCMigrate -I 
/home/atischenko/workspaces/bootstrap/llvm/tools/clang/include -I 
/home/atischenko/workspaces/bootstrap/Release-patch/tools/clang/include -I 
/usr/include/libxml2 -I 
/home/atischenko/workspaces/bootstrap/Release-patch/include -I 
/home/atischenko/workspaces/bootstrap/llvm/include -internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0 
-internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/x86_64-linux-gnu/c++/5.4.0
 -internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/x86_64-linux-gnu/c++/5.4.0
 -internal-isystem 
/usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/backward 
-internal-isystem /usr/local/include -internal-isystem 
/home/atischenko/workspaces/time-report-recursive-4/RelWithDebug/lib/clang/8.0.0/include
 -internal-externc-isystem /usr/include/x86_64-linux-gnu 
-internal-externc-isystem /include -internal-externc-isystem /usr/include 
-Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra 
-Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers 
-Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wstring-conversion -Woverloaded-virtual 
-Wno-nested-anon-types -pedantic -std=c++11 -fdeprecated-macro 
-fdebug-compilation-dir 
/home/atischenko/workspaces/bootstrap/Release-patch/tools/clang/lib/ARCMigrate 
-ferror-limit 19 -fmessage-length 0 -fvisibility-inlines-hidden -ftime-report 
-fno-rtti -fobjc-runtime=gcc -fno-common -fdiagnostics-show-option -o 
TransGCCalls.cpp.o -x c++ 
/home/atischenko/workspaces/bootstrap/llvm/tools/clang/lib/ARCMigrate/TransGCCalls.cpp
 -faddrsig -mllvm -debug-only=ftiming

The full log is too big that's why I grepped "declval" only:

- :start: means the timer was started
- :stop-zero: means the timer was stopped and nothing was added to the 
calculated time period
- :stop-add: means the timer was stoped and the time slice was added to 
the time period.

There are DeclFunc* names and Tags to look for corresponding RAII objects 
inside sources. For example:

ActOnFunctionDeclarator:start:declval:

means

Tag:start:Name:

BTW, I see a lot of broken DeclFunc names: do you have any idea why it's 
possible? Example:

ActOnFunctionDeclarator:start:._exception:

Here  .. means non-ASCII symbols.


https://reviews.llvm.org/D47196



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 160997.
avt77 added a comment.

The ability to produce debug output for 'ftiming' was added. As result now it's 
possible to check places where timers start/stop and for what functions it's 
being done (see changes in Utils.h).


https://reviews.llvm.org/D47196

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.h
  include/clang/Frontend/Utils.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/TreeTransform.h
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/Tooling/Tooling.cpp
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123) + foo(177.2) - foo(331.442);
+  char R2 = foo('d', 1234) * foo(1.26);
+  int R3 = foo(1.2) + foo(11.22) / foo(66.77);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] * foo(15.52) - two[1] / foo(51.25);
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,10 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// FIXME:  We need more complex test to increase the compilation time;
+// otherwise we see the foolowing message from time to time only.
+// VIOLATILE-CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -12,6 +12,8 @@
 //
 //===--===//
 
+#define DEBUG_TYPE "clang-tooling"
+
 #include "clang/Tooling/Tooling.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
@@ -60,8 +62,6 @@
 #include 
 #include 
 
-#define DEBUG_TYPE "clang-tooling"
-
 using namespace clang;
 using namespace tooling;
 
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -11,6 +11,8 @@
 //
 //===--===//
 
+#define DEBUG_TYPE "AnalysisConsumer"
+
 #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
 #include "ModelInjector.h"
 #include "clang/AST/Decl.h"
@@ -48,8 +50,6 @@
 using namespace clang;
 using namespace ento;
 
-#define DEBUG_TYPE "AnalysisConsumer"
-
 static std::unique_ptr CreateUbiViz();
 
 STATISTIC(NumFunctionTopLevel, "The # of functions at top level.");
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -10960,6 +10961,11 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  FrontendTimeRAII FTRAII(
+  "TransformLambdaExpr", FrontendTimesIsEnabled,
+  getFrontendFunctionTimeCtx(),
+  {NewCallOperator, 0.0});
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = 

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D45619#1192803, @craig.topper wrote:

> I'm unclear why the we would want to assign clang's FrontendTimesIsEnabled 
> from inside CodeGenAction. If I'm understanding the intentions here, the goal 
> was to add more timing infrastructure to clang. But if the enabling is tied 
> to CodeGenAction, then doesn't that mean any new clang timers wouldn't work 
> under -fsyntax-only?


Historically, it was done to get compilation times for separate files (to find 
the longest times). Because of that I did not assume usage of -fsyntax-only. Is 
it important? Should I move this activity into another place? If YES could you 
give me a hint where it should be done?


Repository:
  rL LLVM

https://reviews.llvm.org/D45619



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D47196#1186460, @efriedma wrote:

> "0.0040" is four milliseconds?  You're probably crediting time incorrectly, 
> somehow.  Can you tell which FrontendTimeRAII the time is coming from?


Not sure I understand you. What do you like to know exactly? Please, clarify.


https://reviews.llvm.org/D47196



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 158542.
avt77 added a comment.

efriedma, I removed redundant RAII objects but I still have the following:

1.0800 (271) _ZSt7declvalv (*)
 1.0840 (273) _ZSt7declvalv (*)
 1.0880 (269) _ZSt7declvalv (*)
 1.1000 (276) _ZSt7declvalv (*)
 1.1200 (282) _ZSt7declvalv (*)
 1.1360 (279) _ZSt7declvalv (*)
 1.1440 (286) _ZSt7declvalv (*)
 1.1760 (292) _ZSt7declvalv (*)
 1.1760 (295) _ZSt7declvalv (*)
 1.1800 (294) _ZSt7declvalv (*)
 1.1880 (298) _ZSt7declvalv (*)
 1.5960 (397) _ZSt7declvalv (*)

Every line corresponds to one unit and the number in parenthesis means the 
number of times we're dealing with the given function during the compilation of 
one unit. You say it's impossible but one time invocation of this function 
costs 0.0040 and 397 times invocations cost 1.588 (very close to the above 
number).

Do you have any ideas how to deal with all this stuff? Should I add/remove 
RAIIs somewhere?


https://reviews.llvm.org/D47196

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.h
  include/clang/Frontend/Utils.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123) + foo(177.2) - foo(331.442);
+  char R2 = foo('d', 1234) * foo(1.26);
+  int R3 = foo(1.2) + foo(11.22) / foo(66.77);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] * foo(15.52) - two[1] / foo(51.25);
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,10 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// FIXME:  We need more complex test to increase the compilation time;
+// otherwise we see the foolowing message from time to time only.
+// VIOLATILE-CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -11002,6 +11003,18 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  FrontendTimeRAII FTRAII(
+  FrontendTimesIsEnabled,
+  getFrontendFunctionTimeCtx(), {NewCallOperator, 0.0});
+
+//  if (FrontendTimesIsEnabled) {
+//// At this point we're sure we're dealing with some function that's why
+//// we're starting the corresponding time slice. We'll stop it in
+//// Sema::ActOnFinishFunctionBody.
+//getFrontendFunctionTimeCtx()->startFrontendTimer(
+//{NewCallOperator, 0.0});
+//  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -128,6 +128,9 @@
 }
 // Add template arguments from a function template specialization.
 else if (FunctionDecl *Function = dyn_cast(Ctx)) {
+  FrontendTimeRAII FTRAII(
+  

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 158221.
avt77 added a comment.
Herald added a subscriber: eraman.

Accordingly to efriedma suggestion I removed start/stopFrontendTimer where it's 
possible and inserted FrontendTimeRAII in several new places. As result the 
patch becomes bigger and bigger. And as another result I got output like here 
(on compiler bootstrap):

 0.5920 (165) _ZSt7declvalv (*)
 0.5960 (155) _ZSt7declvalv (*)
 0.5960 (162) _ZSt7declvalv (*)
 0.6000 (167) _ZSt7declvalv (*)
 0.6040 (155) _ZSt7declvalv (*)
 0.6040 (160) _ZSt7declvalv (*)
 0.6040 (169) _ZSt7declvalv (*)
 
(the above is grep output from build log file)

It means we compile the same function many times inside many files and it 
should be optimised in some way.
Another thought: I should add "human" names to simplify the mangling names 
resolution.


https://reviews.llvm.org/D47196

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.h
  include/clang/Frontend/Utils.h
  lib/AST/Decl.cpp
  lib/AST/Expr.cpp
  lib/Analysis/AnalysisDeclContext.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123) + foo(177.2) - foo(331.442);
+  char R2 = foo('d', 1234) * foo(1.26);
+  int R3 = foo(1.2) + foo(11.22) / foo(66.77);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] * foo(15.52) - two[1] / foo(51.25);
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,10 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// FIXME:  We need more complex test to increase the compilation time;
+// otherwise we see the foolowing message from time to time only.
+// VIOLATILE-CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -11002,6 +11003,18 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  FrontendTimeRAII FTRAII(
+  FrontendTimesIsEnabled,
+  getFrontendFunctionTimeCtx(), {NewCallOperator, 0.0});
+
+//  if (FrontendTimesIsEnabled) {
+//// At this point we're sure we're dealing with some function that's why
+//// we're starting the corresponding time slice. We'll stop it in
+//// Sema::ActOnFinishFunctionBody.
+//getFrontendFunctionTimeCtx()->startFrontendTimer(
+//{NewCallOperator, 0.0});
+//  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -128,6 +128,9 @@
 }
 // Add template arguments from a function template specialization.
 else if (FunctionDecl *Function = dyn_cast(Ctx)) {
+  FrontendTimeRAII FTRAII(
+  

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added inline comments.



Comment at: lib/Sema/SemaLambda.cpp:1447
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{LSI.CallOperator, 0.0});
+  }

efriedma wrote:
> This seems sort of late?  You're starting the timer after the body has 
> already been parsed.
Yes and no but if we decided to extend usage of RAII objects - it deos not 
matter. I'll change it.



Comment at: lib/Sema/TreeTransform.h:11011
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }

efriedma wrote:
> What happens if we never hit ActOnFinishFunctionBody()?  TransformLambdaExpr 
> has an early return if the body doesn't typecheck.
> 
> More generally, given that we have early returns all over the place in Sema, 
> I would be more comfortable using the RAII helper, rather than explicitly 
> calling start/stop, even if that means you have to insert FrontendTimeRAII 
> variables in half a dozen different places in the parser.  (Note I'm 
> specifically talking about the parser here; the explicit stopFrontendTimer in 
> ~CodeGenFunction seems fine.)
It seems you're right: usage of RAII objects is safely. OK, I'll change it.


https://reviews.llvm.org/D47196



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

Ping!

efriedma, do you have any other comments/requirements?


https://reviews.llvm.org/D47196



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 156064.
avt77 added a comment.

I added required comments and did the required changes.


https://reviews.llvm.org/D47196

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.h
  include/clang/Frontend/Utils.h
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123) + foo(177.2) - foo(331.442);
+  char R2 = foo('d', 1234) * foo(1.26);
+  int R3 = foo(1.2) + foo(11.22) / foo(66.77);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] * foo(15.52) - two[1] / foo(51.25);
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,10 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// FIXME:  We need more complex test to increase the compilation time;
+// otherwise we see the foolowing message from time to time only.
+// VIOLATILE-CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -11002,6 +11003,14 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  if (FrontendTimesIsEnabled) {
+// At this point we're sure we're dealing with some function that's why
+// we're starting the corresponding time slice. We'll stop it in
+// Sema::ActOnFinishFunctionBody.
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,17 +10,18 @@
 //  This file implements semantic analysis for C++ lambda expressions.
 //
 //===--===//
-#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/SemaLambda.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
-#include "clang/Sema/SemaLambda.h"
 using namespace clang;
 using namespace sema;
 
@@ -1437,6 +1438,14 @@
 ExprResult Sema::ActOnLambdaExpr(SourceLocation StartLoc, Stmt *Body, 
  Scope *CurScope) {
   LambdaScopeInfo LSI = *cast(FunctionScopes.back());
+
+  if (FrontendTimesIsEnabled) {
+// We're dealing with lambda-function that's why we're starting
+// the corresponding time slice. It will be finished in
+// ActOnFinishFunctionBody.
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{LSI.CallOperator, 0.0});
+  }
   ActOnFinishFunctionBody(LSI.CallOperator, Body);
   return BuildLambdaExpr(StartLoc, Body->getLocEnd(), );
 }
Index: lib/Sema/SemaDecl.cpp

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D47196#1162321, @efriedma wrote:

> Adding startFrontendTimer/stopFrontendTimer helps a little, but it's still 
> difficult to match a given startFrontendTimer to the corresponding 
> stopFrontendTimer because they're in completely different functions in some 
> cases.  Do they really need to be scattered like that?  If they do, please 
> add comments so someone reading the code can match them up.


I'll do it.

> Does getFrontendFunctionTimeCtx need to be a template, given it's always 
> instantiated with `const FunctionDecl *`?

Initially there were 2 types of instantiations: std::string and const 
FunctionDecl *.  Trying to simplify the patch (and to make it shorter) I 
removed string instantiations (hoping to return them back later). And I thought 
it could be useful to have some other types of instantiations in the future. Do 
you think it's redundant? I used std::string in places where we don't have 
FunctionDecl yet. And this counter infrastructure could be used in general with 
any other data types.




Comment at: include/clang/Frontend/Utils.h:338
+  }
+  if (isFirstValid(Result.first) && Result.second > 0.1) {
+FrontendTimes.push_back(Result);

efriedma wrote:
> Why 0.1? Should it be configurable?
Yes, of course. I'll do it.



Comment at: include/clang/Frontend/Utils.h:385
+  range.first++;
+}
+FinalTimes.push_back({E, dist});

efriedma wrote:
> This is merging together the times for functions with the same name? Why is 
> that necessary?
> 
> This function could use a few comments describing what it's doing.
We merge times for the funcs with the same name because we'd like to get the 
total time spending for every function. The idea of this report is to collect 
all time pieces for separate functions and to show the longest times. As result 
we'll be able to see functions with longest compilation times and that's 
exactly our goal.
And, yes I'll add comments soon.


https://reviews.llvm.org/D47196



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 154802.
avt77 added a comment.

I fixed all issues raised by efriedma: GlobalDecl(FD), function body, class 
names, etc. Many tnx for your help.


https://reviews.llvm.org/D47196

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123) + foo(177.2) - foo(331.442);
+  char R2 = foo('d', 1234) * foo(1.26);
+  int R3 = foo(1.2) + foo(11.22) / foo(66.77);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0]*foo(15.52) - two[1]/foo(51.25);
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,10 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// FIXME:  We need more complex test to increase the compilation time;
+// otherwise we see the foolowing message from time to time only.
+// VIOLATILE-CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -10951,6 +10952,11 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,17 +10,18 @@
 //  This file implements semantic analysis for C++ lambda expressions.
 //
 //===--===//
-#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/SemaLambda.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
-#include "clang/Sema/SemaLambda.h"
 using namespace clang;
 using namespace sema;
 
@@ -1433,6 +1434,11 @@
 ExprResult Sema::ActOnLambdaExpr(SourceLocation StartLoc, Stmt *Body, 
  Scope *CurScope) {
   LambdaScopeInfo LSI = *cast(FunctionScopes.back());
+
+  if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{LSI.CallOperator, 0.0});
+  }
   ActOnFinishFunctionBody(LSI.CallOperator, Body);
   return BuildLambdaExpr(StartLoc, Body->getLocEnd(), );
 }
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -28,6 +28,7 @@
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearch.h" // TODO: Sema shouldn't depend on Lex
 #include "clang/Lex/Lexer.h" // TODO: Extract static functions to fix layering.
 #include 

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D47196#1134185, @efriedma wrote:

> Can you give an example of what the output looks like?


First of all excuse me for such delay with my answer: I was on my vacations.
During bootstrap we could see something like here:

[ 84%] Building CXX object 
tools/clang/lib/StaticAnalyzer/Core/CMakeFiles/clangStaticAnalyzerCore.dir/BlockCounter.cpp.o

 Clang Timers: CodeGen Functions 
-

0.0080 (2) 
_ZN5clang7tooling20getOccurrencesOfUSRsEN4llvm8ArrayRefINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcENS1_9StringRefEPNS_4DeclE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling12_GLOBAL__N_115RenameLocFinderEE12TraverseAttrEPNS_4AttrE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling12_GLOBAL__N_115RenameLocFinderEE29TraverseOMPTaskyieldDirectiveEPNS_21OMPTaskyieldDirectiveEPN4llvm15SmallVectorImplINS7_14PointerIntPairIPNS_4StmtELj1EbNS7_21PointerLikeTypeTraitsISB_EENS7_18PointerIntPairInfoISB_Lj1ESD_EE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling22RecursiveSymbolVisitorINS1_12_GLOBAL__N_123USRLocFindingASTVisitor16TraverseBinCommaEPNS_14BinaryOperatorEPN4llvm15SmallVectorImplINS9_14PointerIntPairIPNS_4StmtELj1EbNS9_21PointerLikeTypeTraitsISD_EENS9_18PointerIntPairInfoISD_Lj1ESF_EE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling22RecursiveSymbolVisitorINS1_12_GLOBAL__N_123USRLocFindingASTVisitor21TraverseTypeAliasDeclEPNS_13TypeAliasDeclE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling22RecursiveSymbolVisitorINS1_12_GLOBAL__N_123USRLocFindingASTVisitor22TraverseObjCObjectTypeEPNS_14ObjCObjectTypeE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling12_GLOBAL__N_115RenameLocFinderEE12TraverseDeclEPNS_4DeclE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling12_GLOBAL__N_115RenameLocFinderEE37TraverseVarTemplateSpecializationDeclEPNS_29VarTemplateSpecializationDeclE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling12_GLOBAL__N_115RenameLocFinderEE13TraverseBinGTEPNS_14BinaryOperatorEPN4llvm15SmallVectorImplINS7_14PointerIntPairIPNS_4StmtELj1EbNS7_21PointerLikeTypeTraitsISB_EENS7_18PointerIntPairInfoISB_Lj1ESD_EE
 (*)
 0.0080 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling12_GLOBAL__N_115RenameLocFinderEE31TraverseCXXPseudoDestructorExprEPNS_23CXXPseudoDestructorExprEPN4llvm15SmallVectorImplINS7_14PointerIntPairIPNS_4StmtELj1EbNS7_21PointerLikeTypeTraitsISB_EENS7_18PointerIntPairInfoISB_Lj1ESD_EE
 (*)
 0.0120 (2) 
_ZN5clang7tooling25createRenameAtomicChangesEN4llvm8ArrayRefINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcENS1_9StringRefEPNS_4DeclE
 (*)
 0.0120 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling22RecursiveSymbolVisitorINS1_12_GLOBAL__N_123USRLocFindingASTVisitor15TraverseTypeLocENS_7TypeLocE
 (*)
 0.0160 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling12_GLOBAL__N_115RenameLocFinderEE13PostVisitStmtEPNS_4StmtE
 (*)
 0.0200 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling22RecursiveSymbolVisitorINS1_12_GLOBAL__N_123USRLocFindingASTVisitor12TraverseAttrEPNS_4AttrE
 (*)
 0.0200 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling22RecursiveSymbolVisitorINS1_12_GLOBAL__N_123USRLocFindingASTVisitor13PostVisitStmtEPNS_4StmtE
 (*)
 0.0400 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling12_GLOBAL__N_115RenameLocFinderEE16dataTraverseNodeEPNS_4StmtEPN4llvm15SmallVectorImplINS7_14PointerIntPairIS6_Lj1EbNS7_21PointerLikeTypeTraitsIS6_EENS7_18PointerIntPairInfoIS6_Lj1ESB_EE
 (*)
 0.0440 (2) 
_ZN5clang19RecursiveASTVisitorINS_7tooling22RecursiveSymbolVisitorINS1_12_GLOBAL__N_123USRLocFindingASTVisitor16dataTraverseNodeEPNS_4StmtEPN4llvm15SmallVectorImplINS9_14PointerIntPairIS8_Lj1EbNS9_21PointerLikeTypeTraitsIS8_EENS9_18PointerIntPairInfoIS8_Lj1ESD_EE
 (*)
 0.0720 (1) _ZN5clang19RecursiveASTVisitor12TraverseAttrEPNS_4AttrE (*)

If we collect the full build log, grep and sort it then we could see the 
longest compilation times like here:

0.3240 (2) 
_ZN4llvm18HexagonInstPrinter16printInstructionEPKNS_6MCInstERNS_11raw_ostreamE 
(*)
 0.4520 (2) 
_ZNK4llvm19AMDGPUMCCodeEmitter27verifyInstructionPredicatesERKNS_6MCInstEm (*)
 0.4680 (2) _ZN12_GLOBAL__N_119SystemZDAGToDAGISel10SelectCodeEPN4llvm6SDNodeE 
(*)
 0.5960 (2) 
_ZL20validateOperandClassRN4llvm18MCParsedAsmOperandEN12_GLOBAL__N_114MatchClassKindE
 (*)
 0.6200 (2) _ZN12_GLOBAL__N_115ARMDAGToDAGISel10SelectCodeEPN4llvm6SDNodeE (*)
 0.7280 (2) _ZN12_GLOBAL__N_115PPCDAGToDAGISel10SelectCodeEPN4llvm6SDNodeE (*)
 0.7920 (2) _ZN12_GLOBAL__N_119AArch64DAGToDAGISel10SelectCodeEPN4llvm6SDNodeE 
(*)
 1.3160 (3) _ZN4llvm19HexagonDAGToDAGISel10SelectCodeEPNS_6SDNodeE (*)
 1.4760 (2) _ZN12_GLOBAL__N_118AMDGPUDAGToDAGISel10SelectCodeEPN4llvm6SDNodeE 
(*)
 5.2520 (2) _ZN12_GLOBAL__N_115X86DAGToDAGISel10SelectCodeEPN4llvm6SDNodeE (*)

> It seems like you've scattered the "start" and "stop" calls all over the 

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 149734.
avt77 added a comment.

To simplify the review I removed all LLVM_DEBUG items - now the patch is really 
shorter than it was before.


https://reviews.llvm.org/D47196

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123);
+  char R2 = foo('d', 1234);
+  int R3 = foo(1.2);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] - two[1];
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,8 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -10945,6 +10946,11 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,17 +10,18 @@
 //  This file implements semantic analysis for C++ lambda expressions.
 //
 //===--===//
-#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/SemaLambda.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
-#include "clang/Sema/SemaLambda.h"
 using namespace clang;
 using namespace sema;
 
@@ -1428,6 +1429,11 @@
 ExprResult Sema::ActOnLambdaExpr(SourceLocation StartLoc, Stmt *Body, 
  Scope *CurScope) {
   LambdaScopeInfo LSI = *cast(FunctionScopes.back());
+
+  if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{LSI.CallOperator, 0.0});
+  }
   ActOnFinishFunctionBody(LSI.CallOperator, Body);
   return BuildLambdaExpr(StartLoc, Body->getLocEnd(), );
 }
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -28,6 +28,7 @@
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearch.h" // TODO: Sema shouldn't depend on Lex
 #include "clang/Lex/Lexer.h" // TODO: Extract static functions to fix layering.
 #include "clang/Lex/ModuleLoader.h" // TODO: Sema shouldn't depend on Lex
@@ -8285,6 +8286,10 @@
   isVirtualOkay);
   if (!NewFD) return nullptr;
 
+  FrontendTimeRAII FTRAII(
+  FrontendTimesIsEnabled,
+  

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

Hi All,
What should I do to simplify the review?
I could remove all LLVM_DEBUG related stuff; I could remove all addtional 
counters and leave only necessary one or two of them. As result the patch will 
become shorter.

Should I do all these truncations to simplify the review?
Or maybe I should add comments and/or docs in Utils.h describing the new 
infrastructure?
Maybe you need examples of the produced output?


https://reviews.llvm.org/D47196



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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 148395.
avt77 added a comment.

The sources were re-based to fit in LLVM_DEBUG rename.
One more counter was added.
Debug logging was improved again.


https://reviews.llvm.org/D47196

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123);
+  char R2 = foo('d', 1234);
+  int R3 = foo(1.2);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] - two[1];
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,8 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -10945,6 +10946,11 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1,4 +1,5 @@
-//===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
+//===--- SemaTemplateInstantiateDecl.cpp - C++ T--emplate Decl Instantiation
+//===/
 //
 // The LLVM Compiler Infrastructure
 //
@@ -9,7 +10,6 @@
 //  This file implements C++ template instantiation for declarations.
 //
 //===--===/
-#include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
@@ -20,11 +20,14 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateInstCallback.h"
 
+#define DEBUG_TYPE "templateinst"
 using namespace clang;
 
 static bool isDeclWithinFunction(const Decl *D) {
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,20 +10,23 @@
 //  This file implements semantic analysis for C++ lambda expressions.
 //
 //===--===//
-#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/SemaLambda.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include 

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 148226.
avt77 added a comment.

- switched to llvm::sort  - from std::sort (tnx to mgrang)
- added a new counter (hope it will improve the output numbers)
- slightly changed a couple of existing counters
- seriously improved debug/monitor logging


https://reviews.llvm.org/D47196

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123);
+  char R2 = foo('d', 1234);
+  int R3 = foo(1.2);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] - two[1];
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,8 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -10945,6 +10946,11 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1,4 +1,5 @@
-//===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
+//===--- SemaTemplateInstantiateDecl.cpp - C++ T--emplate Decl Instantiation
+//===/
 //
 // The LLVM Compiler Infrastructure
 //
@@ -9,7 +10,6 @@
 //  This file implements C++ template instantiation for declarations.
 //
 //===--===/
-#include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
@@ -20,11 +20,14 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateInstCallback.h"
 
+#define DEBUG_TYPE "templateinst"
 using namespace clang;
 
 static bool isDeclWithinFunction(const Decl *D) {
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,20 +10,23 @@
 //  This file implements semantic analysis for C++ lambda expressions.
 //
 //===--===//
-#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/SemaLambda.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
+#include 

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 created this revision.
avt77 added reviewers: RKSimon, simon.f.whittaker, mgorny, russell.gallop, 
efriedma, rsmith, mzolotukhin, davezarzycki, vsk.
Herald added a subscriber: mgrang.

I introduce the new infrastructure supporting recursive timers in Clang. This 
patch is based on https://reviews.llvm.org/D45619 [Time-report] (1) and it is 
not the last one in this series. The main idea is a possibilty to have 
"horizontal" timers instead of "vertical" ones which we have today. The patch 
alows to find the most critical functions (from compilation time of view) and 
to to analyze them.


https://reviews.llvm.org/D47196

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar){
+  T Result = bar * bar + bar/1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2){
+  T Result = bar2 * bar + bar/1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  intR1 = foo(123);
+  char   R2 = foo('d', 1234);
+  intR3 = foo(1.2);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] - two[1];
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,8 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -10945,6 +10946,13 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  if (FrontendTimesIsEnabled) {
+llvm::dbgs() << "startFrontendTimer(TransformLambdaExpr): "
+ << NewCallOperator << "\n";
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1,4 +1,5 @@
-//===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
+//===--- SemaTemplateInstantiateDecl.cpp - C++ T--emplate Decl Instantiation
+//===/
 //
 // The LLVM Compiler Infrastructure
 //
@@ -9,7 +10,6 @@
 //  This file implements C++ template instantiation for declarations.
 //
 //===--===/
-#include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
@@ -20,11 +20,14 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateInstCallback.h"
 
+#define DEBUG_TYPE "templateinst"
 using namespace clang;
 
 static bool isDeclWithinFunction(const Decl *D) {
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,20 +10,23 @@
 //  This file implements 

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D45619#1075677, @bjope wrote:

> buildbots have been failing all day (and were still failing). So I pushed my 
> suggested fix.
>  I hope that was OK.


Thank you. It seems everything is OK now.


Repository:
  rL LLVM

https://reviews.llvm.org/D45619



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D45619#1075437, @bjope wrote:

> I can't see that it has been reverted.
>  But I guess that the table maybe is sorted based on time spent in each pass? 
> So that is why it might be sorted differently on different buildbots (or when 
> using pipe etc).
>
> So I think a quick fix is to add -DAG to the checks that can be reorder and 
> submit that fix.


I don't see revert as well. But I did the following:

svn merge -c -330571 .

And everything was OK.


Repository:
  rL LLVM

https://reviews.llvm.org/D45619



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D45619#1075385, @bjope wrote:

> Anyway, if the order isn't deteministic, then a solution could be to use 
> CHECK-DAG instead of CHECK for the checks that may be reordered. For example:


Thank you very much! I'll try to fix it in this way.


Repository:
  rL LLVM

https://reviews.llvm.org/D45619



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D45619#1075260, @thakis wrote:

> In any case, when you see a test failing on bots and the fix isn't obvious,
>  revert first to get the bots back green.


I reverted it immediatly.


Repository:
  rL LLVM

https://reviews.llvm.org/D45619



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 reopened this revision.
avt77 added a comment.
This revision is now accepted and ready to land.

It's terrible but my new test was failed again as result of commit of this 
patch!

///b/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/test/Frontend/ftime-report-template-decl.cpp:155:11:
 error: expected string not found in input
// CHECK: Code Generation Time
//  ^

I don't understand how it's possible. The same problem raised when I committed 
D43578 . Obviously, there is a situation when 
this  test work w/o Code Generation and as result this test is fail because 
code generation time is zerro. Could anyone help me? How should I change the 
test? The simplest way is to remove this line but I don't like this idea.


Repository:
  rL LLVM

https://reviews.llvm.org/D45619



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL330571: Use special new Clang flag 
FrontendTimesIsEnabled instead of llvm… (authored by avt77, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45619?vs=143311=143511#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45619

Files:
  cfe/trunk/include/clang/Frontend/Utils.h
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
  cfe/trunk/lib/Frontend/CMakeLists.txt
  cfe/trunk/lib/Frontend/FrontendTiming.cpp
  cfe/trunk/test/Frontend/ftime-report-template-decl.cpp

Index: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp
@@ -126,7 +126,7 @@
   Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
-  llvm::TimePassesIsEnabled = TimePasses;
+  FrontendTimesIsEnabled = TimePasses;
 }
 llvm::Module *getModule() const { return Gen->GetModule(); }
 std::unique_ptr takeModule() {
@@ -144,12 +144,12 @@
 
   Context = 
 
-  if (llvm::TimePassesIsEnabled)
+  if (FrontendTimesIsEnabled)
 LLVMIRGeneration.startTimer();
 
   Gen->Initialize(Ctx);
 
-  if (llvm::TimePassesIsEnabled)
+  if (FrontendTimesIsEnabled)
 LLVMIRGeneration.stopTimer();
 }
 
@@ -159,15 +159,15 @@
  "LLVM IR generation of declaration");
 
   // Recurse.
-  if (llvm::TimePassesIsEnabled) {
+  if (FrontendTimesIsEnabled) {
 LLVMIRGenerationRefCount += 1;
 if (LLVMIRGenerationRefCount == 1)
   LLVMIRGeneration.startTimer();
   }
 
   Gen->HandleTopLevelDecl(D);
 
-  if (llvm::TimePassesIsEnabled) {
+  if (FrontendTimesIsEnabled) {
 LLVMIRGenerationRefCount -= 1;
 if (LLVMIRGenerationRefCount == 0)
   LLVMIRGeneration.stopTimer();
@@ -180,12 +180,12 @@
   PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
  Context->getSourceManager(),
  "LLVM IR generation of inline function");
-  if (llvm::TimePassesIsEnabled)
+  if (FrontendTimesIsEnabled)
 LLVMIRGeneration.startTimer();
 
   Gen->HandleInlineFunctionDefinition(D);
 
-  if (llvm::TimePassesIsEnabled)
+  if (FrontendTimesIsEnabled)
 LLVMIRGeneration.stopTimer();
 }
 
@@ -227,15 +227,15 @@
 void HandleTranslationUnit(ASTContext ) override {
   {
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
-if (llvm::TimePassesIsEnabled) {
+if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount += 1;
   if (LLVMIRGenerationRefCount == 1)
 LLVMIRGeneration.startTimer();
 }
 
 Gen->HandleTranslationUnit(C);
 
-if (llvm::TimePassesIsEnabled) {
+if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount -= 1;
   if (LLVMIRGenerationRefCount == 0)
 LLVMIRGeneration.stopTimer();
Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -728,7 +728,7 @@
 
 void EmitAssemblyHelper::EmitAssembly(BackendAction Action,
   std::unique_ptr OS) {
-  TimeRegion Region(llvm::TimePassesIsEnabled ?  : nullptr);
+  TimeRegion Region(FrontendTimesIsEnabled ?  : nullptr);
 
   setCommandLineOpts(CodeGenOpts);
 
@@ -858,7 +858,7 @@
 /// `EmitAssembly` at some point in the future when the default switches.
 void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
 BackendAction Action, std::unique_ptr OS) {
-  TimeRegion Region(llvm::TimePassesIsEnabled ?  : nullptr);
+  TimeRegion Region(FrontendTimesIsEnabled ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
   // The new pass manager always makes a target machine available to passes
Index: cfe/trunk/lib/Frontend/CMakeLists.txt
===
--- cfe/trunk/lib/Frontend/CMakeLists.txt
+++ cfe/trunk/lib/Frontend/CMakeLists.txt
@@ -29,6 +29,7 @@
   FrontendAction.cpp
   FrontendActions.cpp
   FrontendOptions.cpp
+  FrontendTiming.cpp
   HeaderIncludeGen.cpp
   InitHeaderSearch.cpp
   InitPreprocessor.cpp
Index: cfe/trunk/lib/Frontend/FrontendTiming.cpp
===
--- cfe/trunk/lib/Frontend/FrontendTiming.cpp
+++ cfe/trunk/lib/Frontend/FrontendTiming.cpp
@@ -0,0 +1,20 @@
+//===- FronendTiming.cpp - Implements Frontend timing utils  --===//
+//
+// The LLVM Compiler Infrastructure
+//

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 143311.
avt77 added a comment.

I fixed issues raised by efriedma.


https://reviews.llvm.org/D45619

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/Frontend/CMakeLists.txt
  lib/Frontend/FrontendTiming.cpp
  test/Frontend/ftime-report-template-decl.cpp

Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -ftime-report  2>&1 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -fdelayed-template-parsing -DDELAYED_TEMPLATE_PARSING -ftime-report  2>&1 | FileCheck %s
+
+// Template function declarations
+template 
+void foo();
+template 
+void foo();
+
+// Template function definitions.
+template 
+void foo() {}
+
+// Template class (forward) declarations
+template 
+struct A;
+template 
+struct b;
+template 
+struct C;
+template 
+struct D;
+
+// Forward declarations with default parameters?
+template 
+class X1;
+template 
+class X2;
+
+// Forward declarations w/template template parameters
+template  class T>
+class TTP1;
+template  class>
+class TTP2;
+template  class T>
+class TTP5;
+
+// Forward declarations with non-type params
+template 
+class NTP0;
+template 
+class NTP1;
+template 
+class NTP2;
+template 
+class NTP3;
+template 
+class NTP4;
+template 
+class NTP5;
+template 
+class NTP6;
+template 
+class NTP7;
+
+// Template class declarations
+template 
+struct A {};
+template 
+struct B {};
+
+namespace PR6184 {
+namespace N {
+template 
+void bar(typename T::x);
+}
+
+template 
+void N::bar(typename T::x) {}
+}
+
+// This PR occurred only in template parsing mode.
+namespace PR17637 {
+template 
+struct L {
+  template 
+  struct O {
+template 
+static void Fun(U);
+  };
+};
+
+template 
+template 
+template 
+void L::O::Fun(U) {}
+
+void Instantiate() { L<0>::O::Fun(0); }
+}
+
+namespace explicit_partial_specializations {
+typedef char ()[1];
+typedef char ()[2];
+typedef char ()[3];
+typedef char ()[4];
+typedef char ()[5];
+typedef char ()[6];
+
+char one[1];
+char two[2];
+char three[3];
+char four[4];
+char five[5];
+char six[6];
+
+template 
+struct bool_ { typedef int type; };
+template <>
+struct bool_ {};
+
+#define XCAT(x, y) x##y
+#define CAT(x, y) XCAT(x, y)
+#define sassert(_b_) bool_<(_b_)>::type CAT(var, __LINE__);
+
+template 
+struct L {
+  template 
+  struct O {
+template 
+static oneT Fun(U);
+  };
+};
+template 
+template 
+template 
+oneT L::O::Fun(U) { return one; }
+
+template <>
+template <>
+template 
+oneT L<0>::O::Fun(U) { return one; }
+
+void Instantiate() {
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+}
+}
+
+template 
+struct Foo {
+  template 
+  using rebind_alloc = _Other;
+};
+template 
+struct _Wrap_alloc {
+  template 
+  using rebind_alloc = typename Foo<_Alloc>::template rebind_alloc<_Other>;
+  template 
+  using rebind = _Wrap_alloc;
+};
+_Wrap_alloc::rebind w;
+
+// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: LLVM IR Generation Time
+// CHECK: Code Generation Time
+// CHECK: Total
+// CHECK: Clang front-end time report
+// CHECK: Clang front-end timer
+// CHECK: Total
Index: lib/Frontend/FrontendTiming.cpp
===
--- lib/Frontend/FrontendTiming.cpp
+++ lib/Frontend/FrontendTiming.cpp
@@ -0,0 +1,20 @@
+//===- FronendTiming.cpp - Implements Frontend timing utils  --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file keps implementation of frontend timing utils.
+//
+//===--===//
+
+#include "clang/Frontend/Utils.h"
+
+namespace clang {
+
+bool FrontendTimesIsEnabled = false;
+
+}
Index: lib/Frontend/CMakeLists.txt
===
--- lib/Frontend/CMakeLists.txt
+++ lib/Frontend/CMakeLists.txt
@@ -29,6 +29,7 @@
   FrontendAction.cpp
   FrontendActions.cpp
   FrontendOptions.cpp
+  FrontendTiming.cpp
   HeaderIncludeGen.cpp
   InitHeaderSearch.cpp
   InitPreprocessor.cpp
Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -126,7 +126,7 @@
   Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
-  llvm::TimePassesIsEnabled = TimePasses;
+  

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added inline comments.



Comment at: test/Frontend/ftime-report-template-decl.cpp:2
+// RUN: %clang %s -S -o - -ftime-report  2>&1 | FileCheck %s
+// RUN: %clang %s -S -o - -fdelayed-template-parsing 
-DDELAYED_TEMPLATE_PARSING -ftime-report  2>&1 | FileCheck %s
+

efriedma wrote:
> What is this test supposed to be testing?  If you're just checking that we 
> output the timers, this doesn't need to be so complicated.
> 
> We generally prefer to use %clang_cc1 for tests like this.
> 
> Please use -emit-llvm instead of -S if you don't actually need assembly.
You wrote "What is this test supposed to be testing? If you're just checking 
that we output the timers, this doesn't need to be so complicated."
I'm going to use this test to show new timers soon that's why it looks so 
complicated at the first glance. At the moment it simply checking the output of 
the existing counter but later it will show the new counters.


https://reviews.llvm.org/D45619



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

Hi All,
Are there other issues related to this patch?
If NO could anyone give me LGTM? I need this patch committed to continue with 
recursive time counters.


https://reviews.llvm.org/D45619



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 142755.
avt77 added a comment.

I moved FrontendTiming.cpp from lib/Basic/ to lib/Frontend/.


https://reviews.llvm.org/D45619

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/Frontend/CMakeLists.txt
  lib/Frontend/FrontendTiming.cpp
  test/Frontend/ftime-report-template-decl.cpp

Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang %s -S -o - -ftime-report  2>&1 | FileCheck %s
+// RUN: %clang %s -S -o - -fdelayed-template-parsing -DDELAYED_TEMPLATE_PARSING -ftime-report  2>&1 | FileCheck %s
+
+// Template function declarations
+template 
+void foo();
+template 
+void foo();
+
+// Template function definitions.
+template 
+void foo() {}
+
+// Template class (forward) declarations
+template 
+struct A;
+template 
+struct b;
+template 
+struct C;
+template 
+struct D;
+
+// Forward declarations with default parameters?
+template 
+class X1;
+template 
+class X2;
+
+// Forward declarations w/template template parameters
+template  class T>
+class TTP1;
+template  class>
+class TTP2;
+template  class T>
+class TTP5;
+
+// Forward declarations with non-type params
+template 
+class NTP0;
+template 
+class NTP1;
+template 
+class NTP2;
+template 
+class NTP3;
+template 
+class NTP4;
+template 
+class NTP5;
+template 
+class NTP6;
+template 
+class NTP7;
+
+// Template class declarations
+template 
+struct A {};
+template 
+struct B {};
+
+namespace PR6184 {
+namespace N {
+template 
+void bar(typename T::x);
+}
+
+template 
+void N::bar(typename T::x) {}
+}
+
+// This PR occurred only in template parsing mode.
+namespace PR17637 {
+template 
+struct L {
+  template 
+  struct O {
+template 
+static void Fun(U);
+  };
+};
+
+template 
+template 
+template 
+void L::O::Fun(U) {}
+
+void Instantiate() { L<0>::O::Fun(0); }
+}
+
+namespace explicit_partial_specializations {
+typedef char ()[1];
+typedef char ()[2];
+typedef char ()[3];
+typedef char ()[4];
+typedef char ()[5];
+typedef char ()[6];
+
+char one[1];
+char two[2];
+char three[3];
+char four[4];
+char five[5];
+char six[6];
+
+template 
+struct bool_ { typedef int type; };
+template <>
+struct bool_ {};
+
+#define XCAT(x, y) x##y
+#define CAT(x, y) XCAT(x, y)
+#define sassert(_b_) bool_<(_b_)>::type CAT(var, __LINE__);
+
+template 
+struct L {
+  template 
+  struct O {
+template 
+static oneT Fun(U);
+  };
+};
+template 
+template 
+template 
+oneT L::O::Fun(U) { return one; }
+
+template <>
+template <>
+template 
+oneT L<0>::O::Fun(U) { return one; }
+
+void Instantiate() {
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+}
+}
+
+template 
+struct Foo {
+  template 
+  using rebind_alloc = _Other;
+};
+template 
+struct _Wrap_alloc {
+  template 
+  using rebind_alloc = typename Foo<_Alloc>::template rebind_alloc<_Other>;
+  template 
+  using rebind = _Wrap_alloc;
+};
+_Wrap_alloc::rebind w;
+
+// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Code Generation Time
+// CHECK: LLVM IR Generation Time
+// CHECK: Total
+// CHECK: Clang front-end time report
+// CHECK: Clang front-end timer
+// CHECK: Total
Index: lib/Frontend/FrontendTiming.cpp
===
--- lib/Frontend/FrontendTiming.cpp
+++ lib/Frontend/FrontendTiming.cpp
@@ -0,0 +1,20 @@
+//===- FronendTiming.cpp - Implements Frontend timing utils  --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file keps implementation of frontend timing utils.
+//
+//===--===//
+
+#include "llvm/Support/Timer.h"
+
+namespace clang {
+
+bool FrontendTimesIsEnabled = false;
+
+}
Index: lib/Frontend/CMakeLists.txt
===
--- lib/Frontend/CMakeLists.txt
+++ lib/Frontend/CMakeLists.txt
@@ -29,6 +29,7 @@
   FrontendAction.cpp
   FrontendActions.cpp
   FrontendOptions.cpp
+  FrontendTiming.cpp
   HeaderIncludeGen.cpp
   InitHeaderSearch.cpp
   InitPreprocessor.cpp
Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -126,7 +126,7 @@
   Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
-  llvm::TimePassesIsEnabled = TimePasses;
+  

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1068417, @rsmith wrote:

> I think we need to fix the overlap issue as a prerequisite to adding timers 
> with large amounts of overlap, especially self-overlap. Otherwise the numbers 
> will likely do more harm than good, as they will significantly misattribute 
> runtime. Fortunately, I think that should only require some relatively small 
> changes to the LLVM timer infrastructure.


JFYI: the given patch introduces non-recursive (self-overlaps) timers only. I'm 
going to introduce self-overlaps timers when  https://reviews.llvm.org/D45619 
is committed.


https://reviews.llvm.org/D43578



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

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1067995, @rsmith wrote:

> I think https://reviews.llvm.org/D45619 is a good change, and I'd like to see 
> that get committed.


Could you give me LGTM in this case? I'm going to publish "recursive"(ovelaped) 
timers but I'd like to base it on https://reviews.llvm.org/D45619.


https://reviews.llvm.org/D43578



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

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1067768, @rsmith wrote:

> Last time I looked at doing this, I found that LLVM's timer infrastructure 
> was fundamentally unsuitable for adding timers like these to Clang.


Thank you for this answer. As I understand we should close both this review and 
https://reviews.llvm.org/D45619 but you'd like to see something like MS has 
here , 
right?


https://reviews.llvm.org/D43578



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added inline comments.



Comment at: lib/Basic/FrontendTiming.cpp:18
+
+bool FrontendTimesIsEnabled = false;
+

efriedma wrote:
> Why is this in lib/Basic, when the declaration is in include/clang/Frontend/?
Because this library is being linked to all others and as result this global 
variable could be seen in any Clang library w/o any changes in config files. Or 
you mean it should be moved from Frontend? But where? It's a frontend feature 
that's why I put its declaration there.


https://reviews.llvm.org/D45619



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

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 created this revision.
avt77 added reviewers: mgorny, russell.gallop, efriedma, rsmith, thakis, 
davezarzycki, RKSimon, simon.f.whittaker.

To simplify the review and commit of D43578  I 
decided to spilt it in several small parts.

This patch is the first part of the new series of patches. This patch 
introduces a dedicated boolean to deal with -ftime-report Clang switch (instead 
of 'llvm::TimePassesIsEnabled' which was LLVM dependent and potentionally could 
increase Clang compilation time).

Next patch will show usage of llvm::NamedRegionTimer instead of 
llvm::TimeRegion - it will really improve the generating output of the given 
feature.

Then I'll show the new class to deal with reqursive time counters (instead of 
approach shown in CodeGenAction.cpp).

And then we'll try new time counters (Preprocessor, Include Files, Parsing, 
Sema, CodeGen, etc.) The final list of counters will grow in dpendence of 
possible future requirements.

Finally, I'm going to introduce the feature which is similar to this one 
.

Hope, all these efforts could be interesting for many of us.


https://reviews.llvm.org/D45619

Files:
  include/clang/Frontend/Utils.h
  lib/Basic/CMakeLists.txt
  lib/Basic/FrontendTiming.cpp
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  test/Frontend/ftime-report-template-decl.cpp

Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang %s -S -o - -ftime-report  2>&1 | FileCheck %s
+// RUN: %clang %s -S -o - -fdelayed-template-parsing -DDELAYED_TEMPLATE_PARSING -ftime-report  2>&1 | FileCheck %s
+
+// Template function declarations
+template 
+void foo();
+template 
+void foo();
+
+// Template function definitions.
+template 
+void foo() {}
+
+// Template class (forward) declarations
+template 
+struct A;
+template 
+struct b;
+template 
+struct C;
+template 
+struct D;
+
+// Forward declarations with default parameters?
+template 
+class X1;
+template 
+class X2;
+
+// Forward declarations w/template template parameters
+template  class T>
+class TTP1;
+template  class>
+class TTP2;
+template  class T>
+class TTP5;
+
+// Forward declarations with non-type params
+template 
+class NTP0;
+template 
+class NTP1;
+template 
+class NTP2;
+template 
+class NTP3;
+template 
+class NTP4;
+template 
+class NTP5;
+template 
+class NTP6;
+template 
+class NTP7;
+
+// Template class declarations
+template 
+struct A {};
+template 
+struct B {};
+
+namespace PR6184 {
+namespace N {
+template 
+void bar(typename T::x);
+}
+
+template 
+void N::bar(typename T::x) {}
+}
+
+// This PR occurred only in template parsing mode.
+namespace PR17637 {
+template 
+struct L {
+  template 
+  struct O {
+template 
+static void Fun(U);
+  };
+};
+
+template 
+template 
+template 
+void L::O::Fun(U) {}
+
+void Instantiate() { L<0>::O::Fun(0); }
+}
+
+namespace explicit_partial_specializations {
+typedef char ()[1];
+typedef char ()[2];
+typedef char ()[3];
+typedef char ()[4];
+typedef char ()[5];
+typedef char ()[6];
+
+char one[1];
+char two[2];
+char three[3];
+char four[4];
+char five[5];
+char six[6];
+
+template 
+struct bool_ { typedef int type; };
+template <>
+struct bool_ {};
+
+#define XCAT(x, y) x##y
+#define CAT(x, y) XCAT(x, y)
+#define sassert(_b_) bool_<(_b_)>::type CAT(var, __LINE__);
+
+template 
+struct L {
+  template 
+  struct O {
+template 
+static oneT Fun(U);
+  };
+};
+template 
+template 
+template 
+oneT L::O::Fun(U) { return one; }
+
+template <>
+template <>
+template 
+oneT L<0>::O::Fun(U) { return one; }
+
+void Instantiate() {
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+}
+}
+
+template 
+struct Foo {
+  template 
+  using rebind_alloc = _Other;
+};
+template 
+struct _Wrap_alloc {
+  template 
+  using rebind_alloc = typename Foo<_Alloc>::template rebind_alloc<_Other>;
+  template 
+  using rebind = _Wrap_alloc;
+};
+_Wrap_alloc::rebind w;
+
+// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Code Generation Time
+// CHECK: LLVM IR Generation Time
+// CHECK: Total
+// CHECK: Clang front-end time report
+// CHECK: Clang front-end timer
+// CHECK: Total
Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -126,7 +126,7 @@
   Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts,
 CodeGenOpts, C, CoverageInfo)),
   LinkModules(std::move(LinkModules)) {
-  llvm::TimePassesIsEnabled = TimePasses;
+  FrontendTimesIsEnabled = TimePasses;
 }
 llvm::Module *getModule() const { return 

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1065827, @davezarzycki wrote:

> It wasn't my suggestion. @thakis wrote: "We probably should have a separate 
> bool in clang and key this off that and make -ftime-report set both."


Sorry, but it's done now. Any comments, suggestions, requirements?


https://reviews.llvm.org/D43578



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

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 142198.
avt77 added a reviewer: davezarzycki.
avt77 added a comment.
Herald added a subscriber: mgorny.

I removed the dependence on TimePassesIsEnabled (as @davezarzycki sugested) and 
fixed the issue with failed test (tnx to @russell.gallop). As result the patch 
was redesigned.


https://reviews.llvm.org/D43578

Files:
  include/clang/Frontend/FrontendAction.h
  include/clang/Frontend/Utils.h
  include/clang/Lex/HeaderSearch.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Basic/CMakeLists.txt
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/Frontend/ASTMerge.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Lex/HeaderSearch.cpp
  lib/Lex/PPMacroExpansion.cpp
  lib/Lex/Pragma.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Parse/Parser.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaTemplate.cpp
  test/Frontend/ftime-report-template-decl.cpp

Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -0,0 +1,161 @@
+// RUN: %clang %s -S -o - -ftime-report  2>&1 | FileCheck %s
+// RUN: %clang %s -S -o - -fdelayed-template-parsing -DDELAYED_TEMPLATE_PARSING -ftime-report  2>&1 | FileCheck %s
+
+// Template function declarations
+template 
+void foo();
+template 
+void foo();
+
+// Template function definitions.
+template 
+void foo() {}
+
+// Template class (forward) declarations
+template 
+struct A;
+template 
+struct b;
+template 
+struct C;
+template 
+struct D;
+
+// Forward declarations with default parameters?
+template 
+class X1;
+template 
+class X2;
+
+// Forward declarations w/template template parameters
+template  class T>
+class TTP1;
+template  class>
+class TTP2;
+template  class T>
+class TTP5;
+
+// Forward declarations with non-type params
+template 
+class NTP0;
+template 
+class NTP1;
+template 
+class NTP2;
+template 
+class NTP3;
+template 
+class NTP4;
+template 
+class NTP5;
+template 
+class NTP6;
+template 
+class NTP7;
+
+// Template class declarations
+template 
+struct A {};
+template 
+struct B {};
+
+namespace PR6184 {
+namespace N {
+template 
+void bar(typename T::x);
+}
+
+template 
+void N::bar(typename T::x) {}
+}
+
+// This PR occurred only in template parsing mode.
+namespace PR17637 {
+template 
+struct L {
+  template 
+  struct O {
+template 
+static void Fun(U);
+  };
+};
+
+template 
+template 
+template 
+void L::O::Fun(U) {}
+
+void Instantiate() { L<0>::O::Fun(0); }
+}
+
+namespace explicit_partial_specializations {
+typedef char ()[1];
+typedef char ()[2];
+typedef char ()[3];
+typedef char ()[4];
+typedef char ()[5];
+typedef char ()[6];
+
+char one[1];
+char two[2];
+char three[3];
+char four[4];
+char five[5];
+char six[6];
+
+template 
+struct bool_ { typedef int type; };
+template <>
+struct bool_ {};
+
+#define XCAT(x, y) x##y
+#define CAT(x, y) XCAT(x, y)
+#define sassert(_b_) bool_<(_b_)>::type CAT(var, __LINE__);
+
+template 
+struct L {
+  template 
+  struct O {
+template 
+static oneT Fun(U);
+  };
+};
+template 
+template 
+template 
+oneT L::O::Fun(U) { return one; }
+
+template <>
+template <>
+template 
+oneT L<0>::O::Fun(U) { return one; }
+
+void Instantiate() {
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+}
+}
+
+template 
+struct Foo {
+  template 
+  using rebind_alloc = _Other;
+};
+template 
+struct _Wrap_alloc {
+  template 
+  using rebind_alloc = typename Foo<_Alloc>::template rebind_alloc<_Other>;
+  template 
+  using rebind = _Wrap_alloc;
+};
+_Wrap_alloc::rebind w;
+
+// CHECK:   = Clang Parser =
+// CHECK:   ---User Time---
+// CHECK:   Parse Top Level Decl
+// CHECK:   Parse Template
+// CHECK:   Parse Function Definition
+// CHECK:   PP Append Macro
+// CHECK:   Scope manipulation
+// CHECK:   PP Find Handler
+// CHECK:   Total
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedTemplate.h"
@@ -32,6 +33,7 @@
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/ManagedStatic.h"
 
 #include 
 using namespace clang;
@@ -187,7 +189,6 @@
   }
 
   QualType ObjectType = ObjectTypePtr.get();
-
   LookupResult R(*this, TName, Name.getLocStart(), LookupOrdinaryName);
   LookupTemplateName(R, S, SS, ObjectType, EnteringContext,
  

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1064113, @russell.gallop wrote:

> We also see an assertion failure prior to the revert. At r329738:


Yes, you're right - thaks again for this test case.


Repository:
  rL LLVM

https://reviews.llvm.org/D43578



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

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1062950, @thakis wrote:

> @davezarzycki remarks in https://reviews.llvm.org/D45485 that this breaks the 
> shared build. The proposed fix there is to make several of clang's modules 
> depend on LLVM's IR library ("Core"). This seems weird to me for two reasons, 
> one architectural, one form a build point of view:
>
> 1. The modules growing the dep don't really depend on IR, they just need that 
> one bool that happens to be defined there. That bool is called 
> `TimePassesIsEnabled` which is a reasonable bool to live in IR, but this 
> patch starts using that bool for meanings other than "should we time 
> passes?". It instead uses the same bool to decide if clang should print a 
> bunch of timing info. We probably should have a separate bool in clang and 
> key this off that and make -ftime-report set both.
> 2. From a build PoV, depending on Core means explicitly depending on TableGen 
> processing the Attributes.td and Intrinsics.td files in include/llvm/IR, 
> which needlessly (explicitly) serializes the build.


In fact the current trunk already depends on TimePassesIsEnabled (w/o this 
patch applied):

//$ find clang -name \*.cpp | xargs grep TimePassesIsEnabled
clang/lib/CodeGen/CodeGenAction.cpp:  llvm::TimePassesIsEnabled = 
TimePasses;
clang/lib/CodeGen/CodeGenAction.cpp:  if (llvm::TimePassesIsEnabled)
clang/lib/CodeGen/CodeGenAction.cpp:  if (llvm::TimePassesIsEnabled)
clang/lib/CodeGen/CodeGenAction.cpp:  if (llvm::TimePassesIsEnabled) {
clang/lib/CodeGen/CodeGenAction.cpp:  if (llvm::TimePassesIsEnabled) {
clang/lib/CodeGen/CodeGenAction.cpp:  if (llvm::TimePassesIsEnabled)
clang/lib/CodeGen/CodeGenAction.cpp:  if (llvm::TimePassesIsEnabled)
clang/lib/CodeGen/CodeGenAction.cpp:if (llvm::TimePassesIsEnabled) {
clang/lib/CodeGen/CodeGenAction.cpp:if (llvm::TimePassesIsEnabled) {
clang/lib/CodeGen/BackendUtil.cpp:  TimeRegion Region(llvm::TimePassesIsEnabled 
?  : nullptr);
clang/lib/CodeGen/BackendUtil.cpp:  TimeRegion Region(llvm::TimePassesIsEnabled 
?  : nullptr);
//
But I agree that such dependence is not OK. I'll create a separate bool instead 
of TimePassesIsEnabled but the question is should I remove the above usage of 
TimePassesIsEnabled as well? Or maybe it should be a separate pre-patch? Or it 
could be left as it is?


Repository:
  rL LLVM

https://reviews.llvm.org/D43578



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

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1063462, @thakis wrote:

> Reverted:


Many thanks for your help!


Repository:
  rL LLVM

https://reviews.llvm.org/D43578



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

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D43578#1063365, @davezarzycki wrote:

> A revert in practice just means undoing the changes. For example, while both 
> `git` and `svn` have `revert` subcommands, you can also just take the 
> original diff/patch, pipe it into `patch -R`, and if there are no conflicts, 
> commit the result.


Unfortunately, revert does not work with committed changes:

revert: Restore pristine working copy state (undo local changes).
usage: revert PATH...

  Revert changes in the working copy at or within PATH, and remove
  conflict markers as well, if any.

- This subcommand does not revert already committed changes.**

But I'll try with patch -R. Tnx.


Repository:
  rL LLVM

https://reviews.llvm.org/D43578



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

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 reopened this revision.
avt77 added a comment.
This revision is now accepted and ready to land.

Obviously, this patch was not ready for commit that's why I reopen the revison.
But I don't know how to revert the patch from trunk: please, help me.


Repository:
  rL LLVM

https://reviews.llvm.org/D43578



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

[PATCH] D43578: -ftime-report switch support in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

Could you help me with revert of the committed patch?
I'll do all necessary changes but I don't know how I can revert the patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D43578



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 abandoned this revision.
avt77 added a comment.

It is not a bug.


https://reviews.llvm.org/D44559



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In fact I think about mul only because of X86 nature: it always put the mul 
result in wider place (than its args). (Don't know about other CPUs - maybe the 
same?) As result we could change the current design to reflect this feature:

8bit x 8bit -> 16bit
16bit x 16bit -> 32bit
...


https://reviews.llvm.org/D44559



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D44559#1049472, @rjmccall wrote:

> You are welcome to start a thread on cfe-dev to gather support for changing 
> the design of -Wconversion to always warn about the potential for overflow in 
> sub-int multiplication.


Maybe the stupid question: what if we introduce another special option 
-Wnoicywarn?


https://reviews.llvm.org/D44559



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

> If that operation overflows, so be it — we're not going to warn about the 
> potential for overflow every time the user adds two ints, and by the same 
> token, it doesn't make any sense to warn about every time the user adds two 
> shorts just because the language made this otherwise-unimportant technical 
> decision to do the arithmetic in a wider type.

There is one comment only: this patch changes the 'mul' operation only and 
initially the change was done for X86 only because it clearly says about 
"doubling" of mul result width. And another comment: currently we have warning 
here:

  int c = a * b;
  T d1 = c; // expected-warning{{implicit conversion loses integer precision: 
'int' to 'char'}}

but we don't have it here:

T d2 = a * b; // when T is char

It's a potential problem from my point of view.
But I don't mind to close the review - and the corresponding bug of course.


https://reviews.llvm.org/D44559



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D44559#1045758, @rjmccall wrote:

> No, I still oppose this patch.


OK, we have 2 possibilities here (fmpov):

1. Forget about the issue and don't do anything now - it is not a bug
2. Return the width based on real analyze of mul args:
  - signed vs. unsigned
  - chars, shorts, etc.

What do you suggest to do? Or maybe there are other variants?


https://reviews.llvm.org/D44559



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

> That's an interesting question.  In general, these warnings do try to ignore 
> the effects of implicit promotion.  We would not want -Wsign-conversion to 
> fire on `unsigned short x = an_unsigned_short + 1;` (or `- 1`, for that 
> matter), even though formally this coerces a `signed int` to `unsigned 
> short`.  Similarly, -Wsign-conversion *should* warn on `signed short x = 
> an_unsigned_short + 1;`, even though formally the promotion from `unsigned 
> short` to `signed int` is not problematic and the final conversion from 
> `signed int` to `signed short` is not a signedness change.  (This second 
> example should also generate a -Wconversion warning, but the questions are 
> independent.)  Applying that strictly here would say that the user is 
> entitled to think of this as an operation on `unsigned char` that then gets 
> losslessly promoted to `signed short`, even though arithmetically that's not 
> what happens.  On the other hand, I do think there's some room for 
> -Wsign-conversion to be more aggressive than -Wconversion about this sort of 
> thing; -Wsign-conversion should generally fire for any changes in signedness 
> from the original operand types (with the usual complexities around constant 
> values), and there's just an exception for computations whose value is known 
> to fit within the expressible range of the result type, which is not true of 
> this multiplication.  So I think it would be acceptable to warn on this.
> 
> John.

OK, is it LGTM?


https://reviews.llvm.org/D44559



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 139271.
avt77 added a comment.

I removed the dependence on arch and updated the tests.


https://reviews.llvm.org/D44559

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/conversion.c
  test/SemaCXX/conversion.cpp


Index: test/SemaCXX/conversion.cpp
===
--- test/SemaCXX/conversion.cpp
+++ test/SemaCXX/conversion.cpp
@@ -298,3 +298,15 @@
   conditional_run_13(NULL);
 }
 }
+
+template
+T mul_t(T a, T b) {
+  int c = a * b;
+  T d1 = c; // expected-warning{{implicit conversion loses integer precision: 
'int' to 'char'}}
+  T d2 = a * b; // expected-warning{{implicit conversion loses integer 
precision: 'int' to 'char'}}
+  return d1 + d2;
+}
+
+char mul_t_test (char a, char b) {
+return mul_t(a, b); // expected-note{{in instantiation of function 
template specialization 'mul_t' requested here}}
+}
Index: test/Sema/conversion.c
===
--- test/Sema/conversion.c
+++ test/Sema/conversion.c
@@ -302,7 +302,7 @@
 }
 
 void test15(char c) {
-  c = c + 1 + c * 2;
+  c = c + 1 + c * 2; // expected-warning {{implicit conversion loses integer 
precision: 'int'}}
   c = (short) c + 1 + c * 2; // expected-warning {{implicit conversion loses 
integer precision}}
 }
 
@@ -448,3 +448,11 @@
   b -= a; // expected-warning {{implicit conversion when assigning computation 
result loses floating-point precision: 'double' to 'float'}}
   return b;
 }
+
+unsigned short foo1(unsigned char a) {
+  return a * a; // expected-warning {{implicit conversion loses integer 
precision: 'int' to 'unsigned short'}}
+}
+
+signed short bar1(signed char a) {
+  return a * a; // expected-warning {{implicit conversion loses integer 
precision: 'int' to 'short'}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8536,8 +8536,13 @@
   return meet;
 }
 
+case BO_Mul: {
+  // The result width should be calculated in advance
+  unsigned opWidth = C.getIntWidth(GetExprType(E));
+  return IntRange(opWidth, false);
+}
+
 // The default behavior is okay for these.
-case BO_Mul:
 case BO_Add:
 case BO_Xor:
 case BO_Or:


Index: test/SemaCXX/conversion.cpp
===
--- test/SemaCXX/conversion.cpp
+++ test/SemaCXX/conversion.cpp
@@ -298,3 +298,15 @@
   conditional_run_13(NULL);
 }
 }
+
+template
+T mul_t(T a, T b) {
+  int c = a * b;
+  T d1 = c; // expected-warning{{implicit conversion loses integer precision: 'int' to 'char'}}
+  T d2 = a * b; // expected-warning{{implicit conversion loses integer precision: 'int' to 'char'}}
+  return d1 + d2;
+}
+
+char mul_t_test (char a, char b) {
+return mul_t(a, b); // expected-note{{in instantiation of function template specialization 'mul_t' requested here}}
+}
Index: test/Sema/conversion.c
===
--- test/Sema/conversion.c
+++ test/Sema/conversion.c
@@ -302,7 +302,7 @@
 }
 
 void test15(char c) {
-  c = c + 1 + c * 2;
+  c = c + 1 + c * 2; // expected-warning {{implicit conversion loses integer precision: 'int'}}
   c = (short) c + 1 + c * 2; // expected-warning {{implicit conversion loses integer precision}}
 }
 
@@ -448,3 +448,11 @@
   b -= a; // expected-warning {{implicit conversion when assigning computation result loses floating-point precision: 'double' to 'float'}}
   return b;
 }
+
+unsigned short foo1(unsigned char a) {
+  return a * a; // expected-warning {{implicit conversion loses integer precision: 'int' to 'unsigned short'}}
+}
+
+signed short bar1(signed char a) {
+  return a * a; // expected-warning {{implicit conversion loses integer precision: 'int' to 'short'}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8536,8 +8536,13 @@
   return meet;
 }
 
+case BO_Mul: {
+  // The result width should be calculated in advance
+  unsigned opWidth = C.getIntWidth(GetExprType(E));
+  return IntRange(opWidth, false);
+}
+
 // The default behavior is okay for these.
-case BO_Mul:
 case BO_Add:
 case BO_Xor:
 case BO_Or:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

>> In https://reviews.llvm.org/D44559#1040799, @rjmccall wrote:
>> 
>>> I think we're correct not to warn here and that GCC/ICC are being noisy.  
>>> The existence of a temporary promotion to a wider type doesn't justify 
>>> warning on arithmetic between two operands that are the same size as the 
>>> ultimate result.  It is totally fair for users to think of this operation 
>>> as being "closed" on the original type.
>> 
>> 
>> Could you please clarify, are you saying that PR35409 
>>  is not a bug, and clang should 
>> continue to not warn in those cases?
> 
> Correct.

Does it mean we should abandon this revision? On the other hand it's a real 
bug, isn't it?


Repository:
  rC Clang

https://reviews.llvm.org/D44559



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

[PATCH] D36492: [time-report] Add preprocessor timer

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added inline comments.



Comment at: include/clang/Lex/PreprocessorOptions.h:165
 public:
-  PreprocessorOptions() : UsePredefines(true), DetailedRecord(false),
+  PreprocessorOptions() : PPTimer("preprocessor", "Preprocessing"),
+  UsePredefines(true),

modocache wrote:
> eduardo-elizondo wrote:
> > Should this be named "Lexing Time" or "Lexing" instead of "Preprocessing"?
> Good idea! Now that the timer's being started up in the `Preprocessor::Lex` 
> method, it probably should be named "Lexing". Alternatively, I could move 
> this into, say,` Lexer::Lex`. I guess there's not much of a distinction in 
> Clang between "lexing" and "preprocessing."
> 
> I had originally picked this name because that's what appears in `gcc 
> -ftime-report`, but I guess we don't need to keep the names the same.
Preprocessing does not means lexing. It includes much more, for example, file 
inclusion, include pathes elaboration, macros expansion, etc. We could merge 
all these parts or we could output the info for evey single piece. In any case 
I think we need a group of timers related to frontend (or maybe several groups: 
for pp, parser, Sema, codegen...). And maybe we need special switches to 
disable/enable the groups (-ftime-report could be used as some super switch for 
all of them). As result we need some repository of the timers and most probably 
this repository should be kept in PreprocessorOpts.

If there are no objections I can take this job on me and come up with new more 
general version of the patch. If it's OK then I'd like to collect the proposes 
about the required details of the time profile: what exactly we'd like to see 
in output? We have gcc output as an initial example but maybe we need more (or 
less) details?



Comment at: lib/Lex/Preprocessor.cpp:746
 void Preprocessor::Lex(Token ) {
+  llvm::TimeRegion(PPOpts->getTimer());
+

vsk wrote:
> MatzeB wrote:
> > modocache wrote:
> > > erik.pilkington wrote:
> > > > Doesn't this just start a timer and immediately end the timer? 
> > > > Shouldn't we do: `llvm::TimeRegion LexTime(PPOpts->getTimer())` so that 
> > > > the dtor gets called when this function returns and we track the time 
> > > > spent in this function?
> > > > 
> > > > Also: this is a pretty hot function, and it looks like TimeRegion does 
> > > > some non-trivial work if time is being tracked. Have you tried testing 
> > > > this on a big c++ file with and without this patch and seeing what the 
> > > > difference in compile time looks like?
> > > Ah, yes you're right! Sorry about that. Actually keeping the timer alive 
> > > for the duration of the method also reveals that the method is called 
> > > recursively, which causes an assert, because timers can't be started 
> > > twice.
> > > 
> > > Another spot in Clang works around this with a "reference counted" timer: 
> > > https://github.com/llvm-mirror/clang/blob/6ac9c51ede0a50cca13dd4ac03562c036f7a3f48/lib/CodeGen/CodeGenAction.cpp#L130-L134.
> > >  I have a more generic version of this "reference counting timer" that 
> > > I've been using for some of the other timers I've been adding; maybe I'll 
> > > use it here as well.
> > FWIF: I share Eriks concerns about compiletime. Timers are enabled in 
> > optimized builds, and querying them is not free. So putting one into a 
> > function that is called a lot and is time critical seems like a bad idea 
> > (do benchmarking to prove or disprove this!).
> The  timer is not started or queried unless -ftime-report is enabled. In the 
> common case the overhead amounts to one extra null-check. And when we're 
> collecting timing information, some performance degradation (say, within 5%) 
> should be acceptable. I agree that we should get a sense for what the 
> overhead is, but am not convinced that this should be a blocking issue.
Here we have more than "one extra null-check": we're dealing with 
constructor/destructor as well (and it's for every Lex()). I agree that it 
should be acceptable for us but we have to profile it (maybe we should not 
profile Lex() but something above it). 



https://reviews.llvm.org/D36492



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

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 closed this revision.
avt77 added a comment.

Fixed by 324721.


https://reviews.llvm.org/D42530



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

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

Committed revision 324721.
BTW, could you review https://reviews.llvm.org/D42728: it's rather similar to 
this one and rather small as well.


https://reviews.llvm.org/D42530



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

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 133404.
avt77 added a comment.

I propagated qualifiers accordingly to rjmccall's suggestion. But I changed the 
diagnostic: now it's more realistic from my point of view.


https://reviews.llvm.org/D42530

Files:
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprMember.cpp
  test/Sema/assign.c
  test/Sema/typedef-retain.c
  test/SemaCXX/err_typecheck_assign_const.cpp

Index: test/SemaCXX/err_typecheck_assign_const.cpp
===
--- test/SemaCXX/err_typecheck_assign_const.cpp
+++ test/SemaCXX/err_typecheck_assign_const.cpp
@@ -129,3 +129,23 @@
   Func ();
   bar()() = 0; // expected-error {{read-only variable is not assignable}}
 }
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+struct OhNo {
+  float4 v;
+  void AssignMe() const { v.x = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo::AssignMe' is declared const here}}
+};
+
+typedef float float4_2 __attribute__((__vector_size__(16)));
+struct OhNo2 {
+  float4_2 v;
+  void AssignMe() const { v[0] = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo2::AssignMe' is declared const here}}
+};
+
+struct OhNo3 {
+  float v[4];
+  void AssignMe() const { v[0] = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo3::AssignMe' is declared const here}}
+};
Index: test/Sema/typedef-retain.c
===
--- test/Sema/typedef-retain.c
+++ test/Sema/typedef-retain.c
@@ -16,8 +16,8 @@
 typedef int a[5];
 void test3() {
   typedef const a b;
-  b r;
-  r[0]=10;  // expected-error {{read-only variable is not assignable}}
+  b r;   // expected-note {{variable 'r' declared const here}}
+  r[0] = 10; // expected-error {{cannot assign to variable 'r' with const-qualified type 'b' (aka 'int const[5]')}}
 }
 
 int test4(const a y) {
Index: test/Sema/assign.c
===
--- test/Sema/assign.c
+++ test/Sema/assign.c
@@ -11,10 +11,10 @@
 
 typedef int arr[10];
 void test3() {
-  const arr b;
-  const int b2[10]; 
-  b[4] = 1; // expected-error {{read-only variable is not assignable}}
-  b2[4] = 1; // expected-error {{read-only variable is not assignable}}
+  const arr b;  // expected-note {{variable 'b' declared const here}}
+  const int b2[10]; // expected-note {{variable 'b2' declared const here}}
+  b[4] = 1; // expected-error {{cannot assign to variable 'b' with const-qualified type 'const arr' (aka 'int const[10]')}}
+  b2[4] = 1;// expected-error {{cannot assign to variable 'b2' with const-qualified type 'const int [10]'}}
 }
 
 typedef struct I {
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1623,10 +1623,14 @@
   else
 VK = BaseExpr.get()->getValueKind();
 }
+
 QualType ret = CheckExtVectorComponent(S, BaseType, VK, OpLoc,
Member, MemberLoc);
 if (ret.isNull())
   return ExprError();
+Qualifiers BaseQ =
+S.Context.getCanonicalType(BaseExpr.get()->getType()).getQualifiers();
+ret = S.Context.getQualifiedType(ret, BaseQ);
 
 return new (S.Context)
 ExtVectorElementExpr(ret, VK, BaseExpr.get(), *Member, MemberLoc);
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -4395,8 +4395,14 @@
 if (VK != VK_RValue)
   OK = OK_VectorComponent;
 
-// FIXME: need to deal with const...
 ResultType = VTy->getElementType();
+QualType BaseType = BaseExpr->getType();
+Qualifiers BaseQuals = BaseType.getQualifiers();
+Qualifiers MemberQuals =
+Context.getCanonicalType(ResultType).getQualifiers();
+Qualifiers Combined = BaseQuals + MemberQuals;
+if (Combined != MemberQuals)
+  ResultType = Context.getQualifiedType(ResultType, Combined);
   } else if (LHSTy->isArrayType()) {
 // If we see an array that wasn't promoted by
 // DefaultFunctionArrayLvalueConversion, it must be an array that
@@ -10434,8 +10440,16 @@
 // Static fields do not inherit constness from parents.
 break;
   }
-  break;
-} // End MemberExpr
+  break; // End MemberExpr
+} else if (const ArraySubscriptExpr *ASE =
+   dyn_cast(E)) {
+  E = ASE->getBase()->IgnoreParenImpCasts();
+  continue;
+} else if (const ExtVectorElementExpr *EVE =
+

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 132607.
avt77 added a comment.

I re-implemented the patch. Now it works properly with const methods in CXX 
classes.


https://reviews.llvm.org/D42530

Files:
  lib/AST/ExprClassification.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/assign.c
  test/Sema/typedef-retain.c
  test/SemaCXX/anonymous-union.cpp
  test/SemaCXX/err_typecheck_assign_const.cpp

Index: test/SemaCXX/err_typecheck_assign_const.cpp
===
--- test/SemaCXX/err_typecheck_assign_const.cpp
+++ test/SemaCXX/err_typecheck_assign_const.cpp
@@ -129,3 +129,23 @@
   Func ();
   bar()() = 0; // expected-error {{read-only variable is not assignable}}
 }
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+struct OhNo {
+  float4 v;
+  void AssignMe() const { v.x = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo::AssignMe' is declared const here}}
+};
+
+typedef float float4_2 __attribute__((__vector_size__(16)));
+struct OhNo2 {
+  float4_2 v;
+  void AssignMe() const { v[0] = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo2::AssignMe' is declared const here}}
+};
+
+struct OhNo3 {
+  float v[4];
+  void AssignMe() const { v[0] = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo3::AssignMe' is declared const here}}
+};
Index: test/SemaCXX/anonymous-union.cpp
===
--- test/SemaCXX/anonymous-union.cpp
+++ test/SemaCXX/anonymous-union.cpp
@@ -40,7 +40,8 @@
 }
 
 void X::test_unqual_references_const() const { // expected-note 2{{member function 'X::test_unqual_references_const' is declared const here}}
-  d = 0.0;
+  // TODO: it seems we should not see any error here becuase 'd' is mutable
+  d = 0.0; // expected-error {{read-only variable is not assignable}}
   f2 = 0; // expected-error{{cannot assign to non-static data member within const member function 'test_unqual_references_const'}}
   a = 0; // expected-error{{cannot assign to non-static data member within const member function 'test_unqual_references_const'}}
 }
Index: test/Sema/typedef-retain.c
===
--- test/Sema/typedef-retain.c
+++ test/Sema/typedef-retain.c
@@ -16,8 +16,8 @@
 typedef int a[5];
 void test3() {
   typedef const a b;
-  b r;
-  r[0]=10;  // expected-error {{read-only variable is not assignable}}
+  b r;  // expected-note {{variable 'r' declared const here}}
+  r[0]=10;  // expected-error {{cannot assign to variable 'r' with const-qualified type 'b' (aka 'int const[5]')}}
 }
 
 int test4(const a y) {
Index: test/Sema/assign.c
===
--- test/Sema/assign.c
+++ test/Sema/assign.c
@@ -11,10 +11,10 @@
 
 typedef int arr[10];
 void test3() {
-  const arr b;
-  const int b2[10]; 
-  b[4] = 1; // expected-error {{read-only variable is not assignable}}
-  b2[4] = 1; // expected-error {{read-only variable is not assignable}}
+  const arr b;  // expected-note {{variable 'b' declared const here}}
+  const int b2[10];   // expected-note {{variable 'b2' declared const here}}
+  b[4] = 1; // expected-error {{cannot assign to variable 'b' with const-qualified type 'const arr' (aka 'int const[10]')}}
+  b2[4] = 1; // expected-error {{cannot assign to variable 'b2' with const-qualified type 'const int [10]'}}
 }
 
 typedef struct I {
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10321,7 +10321,7 @@
 /// 'const' due to being captured within a block?
 enum NonConstCaptureKind { NCCK_None, NCCK_Block, NCCK_Lambda };
 static NonConstCaptureKind isReferenceToNonConstCapture(Sema , Expr *E) {
-  assert(E->isLValue() && E->getType().isConstQualified());
+  assert(E->isLValue());
   E = E->IgnoreParens();
 
   // Must be a reference to a declaration from an enclosing scope.
@@ -10401,10 +10401,8 @@
   const ValueDecl *VD = ME->getMemberDecl();
   if (const FieldDecl *Field = dyn_cast(VD)) {
 // Mutable fields can be modified even if the class is const.
-if (Field->isMutable()) {
-  assert(DiagnosticEmitted && "Expected diagnostic not emitted.");
+if (Field->isMutable())
   break;
-}
 
 if (!IsTypeModifiable(Field->getType(), IsDereference)) {
   if (!DiagnosticEmitted) {
@@ -10434,8 +10432,16 @@
 // Static fields do not inherit constness from parents.
 break;
   }
-  break;
-} // End MemberExpr
+  

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D42530#995227, @rjmccall wrote:

> That's still just const-propagation.  The const qualifier on the pointee type 
> of this should propagate to the l-value resulting from the member access, and 
> the vector-specific projection logic should continue to propagate const to 
> the type of the element l-value.


I'm afraid it's wrong because from my point of view Clang generates wrong 
message for test OhNo3 (see err_typecheck_assign_const.cpp). It should not be  
"read-only variable is not assignable" because v[4] is not read-only variable. 
Any other non-const method is able to change this variable w/o any problems. 
The the same is for OhNo and OhNo2.

BTW, gcc generates

error: assignment of read-only location ‘*(const float*)(&((const 
OhNo2*)this)->OhNo2::v)’

and that's OK because we try to change the value through 
const-pointer-to-const. Am I right?


https://reviews.llvm.org/D42530



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

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In fact we have here another problem: it is not an attempt to assign to const 
variable but it is an attempt to use the field assingment inside const  method: 
I'm working on it.


https://reviews.llvm.org/D42530



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

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added inline comments.



Comment at: lib/AST/ExprClassification.cpp:652
+if (Ctx.getCanonicalType(ASE->getBase()->getType()).isConstQualified())
+  return Cl::CM_ConstQualified;
+

rjmccall wrote:
> Is there a reason why the places that compute the type of these l-value 
> expressions don't propagate qualiifers?  This hardly seems restricted to 
> 'const'.
Do you mean ExtVectorElementExpr/ArraySubscriptExpr should retrun 
isConstQualified() == true by themselves (and other qualifiers as well)?


https://reviews.llvm.org/D42530



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