https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/106277
>From b2bb29ec61f4e9a7b3b7f9bcd0f5b7a12c844d14 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 27 Aug 2024 19:44:34 +0000 Subject: [PATCH 1/4] [clang] Properly set file and line info for -ftime-trace --- clang/lib/Parse/Parser.cpp | 9 +++++++-- ...-trace-ParseDeclarationOrFunctionDefinition.cpp | 3 ++- clang/unittests/Support/TimeProfilerTest.cpp | 14 +++++++------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index 5ebe71e496a2e8..cc16a9d98d0ca6 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -16,6 +16,7 @@ #include "clang/AST/ASTLambda.h" #include "clang/AST/DeclTemplate.h" #include "clang/Basic/FileManager.h" +#include "clang/Basic/SourceManager.h" #include "clang/Parse/ParseDiagnostic.h" #include "clang/Parse/RAIIObjectsForParser.h" #include "clang/Sema/DeclSpec.h" @@ -1255,8 +1256,12 @@ Parser::DeclGroupPtrTy Parser::ParseDeclarationOrFunctionDefinition( // Add an enclosing time trace scope for a bunch of small scopes with // "EvaluateAsConstExpr". llvm::TimeTraceScope TimeScope("ParseDeclarationOrFunctionDefinition", [&]() { - return Tok.getLocation().printToString( - Actions.getASTContext().getSourceManager()); + llvm::TimeTraceMetadata M; + const SourceManager &SM = Actions.getASTContext().getSourceManager(); + auto Loc = SM.getExpansionLoc(Tok.getLocation()); + M.File = SM.getFilename(Loc); + M.Line = SM.getExpansionLineNumber(Loc); + return M; }); if (DS) { diff --git a/clang/test/Driver/check-time-trace-ParseDeclarationOrFunctionDefinition.cpp b/clang/test/Driver/check-time-trace-ParseDeclarationOrFunctionDefinition.cpp index f854cddadbfcc1..8a1127752b325b 100644 --- a/clang/test/Driver/check-time-trace-ParseDeclarationOrFunctionDefinition.cpp +++ b/clang/test/Driver/check-time-trace-ParseDeclarationOrFunctionDefinition.cpp @@ -4,7 +4,8 @@ // RUN: | FileCheck %s // CHECK-DAG: "name": "ParseDeclarationOrFunctionDefinition" -// CHECK-DAG: "detail": "{{.*}}check-time-trace-ParseDeclarationOrFunctionDefinition.cpp:15:1" +// CHECK-DAG: "file": "{{.*}}check-time-trace-ParseDeclarationOrFunctionDefinition.cpp" +// CHECK-DAG: "line": 16 // CHECK-DAG: "name": "ParseFunctionDefinition" // CHECK-DAG: "detail": "foo" // CHECK-DAG: "name": "ParseFunctionDefinition" diff --git a/clang/unittests/Support/TimeProfilerTest.cpp b/clang/unittests/Support/TimeProfilerTest.cpp index f53fe71d630bf5..6d8b4e0453294f 100644 --- a/clang/unittests/Support/TimeProfilerTest.cpp +++ b/clang/unittests/Support/TimeProfilerTest.cpp @@ -210,8 +210,8 @@ constexpr int slow_init_list[] = {1, 1, 2, 3, 5, 8, 13, 21}; // 25th line std::string Json = teardownProfiler(); ASSERT_EQ(R"( Frontend (test.cc) -| ParseDeclarationOrFunctionDefinition (test.cc:2:1) -| ParseDeclarationOrFunctionDefinition (test.cc:6:1) +| ParseDeclarationOrFunctionDefinition (test.cc:2) +| ParseDeclarationOrFunctionDefinition (test.cc:6) | | ParseFunctionDefinition (slow_func) | | | EvaluateAsRValue (<test.cc:8:21>) | | | EvaluateForOverflow (<test.cc:8:21, col:25>) @@ -223,15 +223,15 @@ Frontend (test.cc) | | | | EvaluateAsRValue (<test.cc:8:21, col:25>) | | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>) | | | | EvaluateAsRValue (<test.cc:8:21, col:25>) -| ParseDeclarationOrFunctionDefinition (test.cc:16:1) +| ParseDeclarationOrFunctionDefinition (test.cc:16) | | ParseFunctionDefinition (slow_test) | | | EvaluateAsInitializer (slow_value) | | | EvaluateAsConstantExpr (<test.cc:17:33, col:59>) | | | EvaluateAsConstantExpr (<test.cc:18:11, col:37>) -| ParseDeclarationOrFunctionDefinition (test.cc:22:1) +| ParseDeclarationOrFunctionDefinition (test.cc:22) | | EvaluateAsConstantExpr (<test.cc:23:31, col:57>) | | EvaluateAsRValue (<test.cc:22:14, line:23:58>) -| ParseDeclarationOrFunctionDefinition (test.cc:25:1) +| ParseDeclarationOrFunctionDefinition (test.cc:25) | | EvaluateAsInitializer (slow_init_list) | PerformPendingInstantiations )", @@ -270,7 +270,7 @@ Frontend (test.cc) | ParseFunctionDefinition (fooB) | ParseFunctionDefinition (fooMTA) | ParseFunctionDefinition (fooA) -| ParseDeclarationOrFunctionDefinition (test.cc:3:5) +| ParseDeclarationOrFunctionDefinition (test.cc:3) | | ParseFunctionDefinition (user) | PerformPendingInstantiations | | InstantiateFunction (fooA<int>, a.h:7) @@ -292,7 +292,7 @@ struct { std::string Json = teardownProfiler(); ASSERT_EQ(R"( Frontend (test.c) -| ParseDeclarationOrFunctionDefinition (test.c:2:1) +| ParseDeclarationOrFunctionDefinition (test.c:2) | | isIntegerConstantExpr (<test.c:3:18>) | | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>) | PerformPendingInstantiations >From 126fbc39d71d1ae674cfa2a0b23f994c7420ad9b Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Tue, 27 Aug 2024 20:03:42 +0000 Subject: [PATCH 2/4] Add release notes --- clang/docs/ReleaseNotes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b165f6e65636d9..60d4e78dcc48c4 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -136,6 +136,9 @@ Improvements to Clang's diagnostics Improvements to Clang's time-trace ---------------------------------- +- Clang now adds file and line information for -ftime-trace profile as separate fields + (as ``event["args"]["file"]`` and ``event["args"]["line"]`` respectively). + Improvements to Coverage Mapping -------------------------------- >From fd9f5218e72d0d9b4d4b248383179f6dea01a5c6 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 29 Aug 2024 12:23:26 +0000 Subject: [PATCH 3/4] Address comments: Add col; Calculate loc info in a functionq --- clang/include/clang/Basic/SourceManager.h | 5 ++ clang/lib/AST/ExprConstant.cpp | 7 ++- clang/lib/Basic/SourceManager.cpp | 8 +++ clang/lib/Parse/Parser.cpp | 6 +-- clang/lib/Sema/SemaTemplateInstantiate.cpp | 7 +-- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 7 +-- clang/unittests/Support/TimeProfilerTest.cpp | 54 +++++++++---------- 7 files changed, 51 insertions(+), 43 deletions(-) diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index d3ccc7ef81c079..488bc11715029a 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -50,6 +50,7 @@ #include "llvm/Support/Allocator.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/TimeProfiler.h" #include <cassert> #include <cstddef> #include <map> @@ -906,6 +907,10 @@ class SourceManager : public RefCountedBase<SourceManager> { /// Get the file ID for the precompiled preamble if there is one. FileID getPreambleFileID() const { return PreambleFileID; } + /// Set Location information for Time trace events. + void setLocationForTimeTrace(SourceLocation Loc, + llvm::TimeTraceMetadata &M) const; + //===--------------------------------------------------------------------===// // Methods to create new FileID's and macro expansions. //===--------------------------------------------------------------------===// diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 558e20ed3e4239..9087572d0c4178 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -52,6 +52,7 @@ #include "clang/AST/TypeLoc.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/DiagnosticSema.h" +#include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" #include "llvm/ADT/APFixedPoint.h" #include "llvm/ADT/SmallBitVector.h" @@ -672,12 +673,14 @@ namespace { }; // A shorthand time trace scope struct, prints source range, for example - // {"name":"EvaluateAsRValue","args":{"detail":"<test.cc:8:21, col:25>"}}} + // {"name":"EvaluateAsRValue","args":{"detail":"test.cc", "line":8, "col":21"}}} class ExprTimeTraceScope { public: ExprTimeTraceScope(const Expr *E, const ASTContext &Ctx, StringRef Name) : TimeScope(Name, [E, &Ctx] { - return E->getSourceRange().printToString(Ctx.getSourceManager()); + llvm::TimeTraceMetadata M; + Ctx.getSourceManager().setLocationForTimeTrace(E->getBeginLoc(), M); + return M; }) {} private: diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index 533a9fe88a2150..8dc7bb0d98bd1d 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -1064,6 +1064,14 @@ bool SourceManager::isAtStartOfImmediateMacroExpansion(SourceLocation Loc, return true; } +void SourceManager::setLocationForTimeTrace(SourceLocation Loc, + llvm::TimeTraceMetadata &M) const { + Loc = getExpansionLoc(Loc); + M.File = getFilename(Loc); + M.Line = getExpansionLineNumber(Loc); + M.Col = getExpansionColumnNumber(Loc); +} + bool SourceManager::isAtEndOfImmediateMacroExpansion(SourceLocation Loc, SourceLocation *MacroEnd) const { assert(Loc.isValid() && Loc.isMacroID() && "Expected a valid macro loc"); diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index cc16a9d98d0ca6..d6829045bb4293 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1257,10 +1257,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclarationOrFunctionDefinition( // "EvaluateAsConstExpr". llvm::TimeTraceScope TimeScope("ParseDeclarationOrFunctionDefinition", [&]() { llvm::TimeTraceMetadata M; - const SourceManager &SM = Actions.getASTContext().getSourceManager(); - auto Loc = SM.getExpansionLoc(Tok.getLocation()); - M.File = SM.getFilename(Loc); - M.Line = SM.getExpansionLineNumber(Loc); + Actions.getASTContext().getSourceManager().setLocationForTimeTrace( + Tok.getLocation(), M); return M; }); diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 8995d461362d70..5afd3c0596e612 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -3423,11 +3423,8 @@ Sema::InstantiateClass(SourceLocation PointOfInstantiation, llvm::raw_string_ostream OS(M.Detail); Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(), /*Qualified=*/true); - if (llvm::isTimeTraceVerbose()) { - auto Loc = SourceMgr.getExpansionLoc(Instantiation->getLocation()); - M.File = SourceMgr.getFilename(Loc); - M.Line = SourceMgr.getExpansionLineNumber(Loc); - } + if (llvm::isTimeTraceVerbose()) + SourceMgr.setLocationForTimeTrace(Instantiation->getLocation(), M); return M; }); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index a12d2eff1d2c81..2415616801108b 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -4970,11 +4970,8 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, llvm::raw_string_ostream OS(M.Detail); Function->getNameForDiagnostic(OS, getPrintingPolicy(), /*Qualified=*/true); - if (llvm::isTimeTraceVerbose()) { - auto Loc = SourceMgr.getExpansionLoc(Function->getLocation()); - M.File = SourceMgr.getFilename(Loc); - M.Line = SourceMgr.getExpansionLineNumber(Loc); - } + if (llvm::isTimeTraceVerbose()) + SourceMgr.setLocationForTimeTrace(Function->getLocation(), M); return M; }); diff --git a/clang/unittests/Support/TimeProfilerTest.cpp b/clang/unittests/Support/TimeProfilerTest.cpp index 6d8b4e0453294f..a4cdea003982ec 100644 --- a/clang/unittests/Support/TimeProfilerTest.cpp +++ b/clang/unittests/Support/TimeProfilerTest.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "clang/Frontend/CompilerInstance.h" -#include "clang/Frontend/FrontendActions.h" #include "clang/Lex/PreprocessorOptions.h" #include "llvm/ADT/StringMap.h" @@ -18,7 +17,6 @@ #include <stack> #include "gtest/gtest.h" -#include <tuple> using namespace clang; using namespace llvm; @@ -86,6 +84,8 @@ std::string GetMetadata(json::Object *Event) { OS << (M.empty() ? "" : ", ") << llvm::sys::path::filename(*File); if (auto Line = Args->getInteger("line")) OS << ":" << *Line; + if (auto Col = Args->getInteger("col")) + OS << ":" << *Col; } return M; } @@ -210,28 +210,28 @@ constexpr int slow_init_list[] = {1, 1, 2, 3, 5, 8, 13, 21}; // 25th line std::string Json = teardownProfiler(); ASSERT_EQ(R"( Frontend (test.cc) -| ParseDeclarationOrFunctionDefinition (test.cc:2) -| ParseDeclarationOrFunctionDefinition (test.cc:6) +| ParseDeclarationOrFunctionDefinition (test.cc:2:1) +| ParseDeclarationOrFunctionDefinition (test.cc:6:1) | | ParseFunctionDefinition (slow_func) -| | | EvaluateAsRValue (<test.cc:8:21>) -| | | EvaluateForOverflow (<test.cc:8:21, col:25>) -| | | EvaluateForOverflow (<test.cc:8:30, col:32>) -| | | EvaluateAsRValue (<test.cc:9:14>) -| | | EvaluateForOverflow (<test.cc:9:9, col:14>) +| | | EvaluateAsRValue (test.cc:8:21) +| | | EvaluateForOverflow (test.cc:8:21) +| | | EvaluateForOverflow (test.cc:8:30) +| | | EvaluateAsRValue (test.cc:9:14) +| | | EvaluateForOverflow (test.cc:9:9) | | | isPotentialConstantExpr (slow_namespace::slow_func) -| | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>) -| | | | EvaluateAsRValue (<test.cc:8:21, col:25>) -| | | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>) -| | | | EvaluateAsRValue (<test.cc:8:21, col:25>) -| ParseDeclarationOrFunctionDefinition (test.cc:16) +| | | EvaluateAsBooleanCondition (test.cc:8:21) +| | | | EvaluateAsRValue (test.cc:8:21) +| | | EvaluateAsBooleanCondition (test.cc:8:21) +| | | | EvaluateAsRValue (test.cc:8:21) +| ParseDeclarationOrFunctionDefinition (test.cc:16:1) | | ParseFunctionDefinition (slow_test) | | | EvaluateAsInitializer (slow_value) -| | | EvaluateAsConstantExpr (<test.cc:17:33, col:59>) -| | | EvaluateAsConstantExpr (<test.cc:18:11, col:37>) -| ParseDeclarationOrFunctionDefinition (test.cc:22) -| | EvaluateAsConstantExpr (<test.cc:23:31, col:57>) -| | EvaluateAsRValue (<test.cc:22:14, line:23:58>) -| ParseDeclarationOrFunctionDefinition (test.cc:25) +| | | EvaluateAsConstantExpr (test.cc:17:33) +| | | EvaluateAsConstantExpr (test.cc:18:11) +| ParseDeclarationOrFunctionDefinition (test.cc:22:1) +| | EvaluateAsConstantExpr (test.cc:23:31) +| | EvaluateAsRValue (test.cc:22:14) +| ParseDeclarationOrFunctionDefinition (test.cc:25:1) | | EvaluateAsInitializer (slow_init_list) | PerformPendingInstantiations )", @@ -270,12 +270,12 @@ Frontend (test.cc) | ParseFunctionDefinition (fooB) | ParseFunctionDefinition (fooMTA) | ParseFunctionDefinition (fooA) -| ParseDeclarationOrFunctionDefinition (test.cc:3) +| ParseDeclarationOrFunctionDefinition (test.cc:3:5) | | ParseFunctionDefinition (user) | PerformPendingInstantiations -| | InstantiateFunction (fooA<int>, a.h:7) -| | | InstantiateFunction (fooB<int>, b.h:3) -| | | InstantiateFunction (fooMTA<int>, a.h:4) +| | InstantiateFunction (fooA<int>, a.h:7:10) +| | | InstantiateFunction (fooB<int>, b.h:3:7) +| | | InstantiateFunction (fooMTA<int>, a.h:4:5) )", buildTraceGraph(Json)); } @@ -292,9 +292,9 @@ struct { std::string Json = teardownProfiler(); ASSERT_EQ(R"( Frontend (test.c) -| ParseDeclarationOrFunctionDefinition (test.c:2) -| | isIntegerConstantExpr (<test.c:3:18>) -| | EvaluateKnownConstIntCheckOverflow (<test.c:3:18>) +| ParseDeclarationOrFunctionDefinition (test.c:2:1) +| | isIntegerConstantExpr (test.c:3:18) +| | EvaluateKnownConstIntCheckOverflow (test.c:3:18) | PerformPendingInstantiations )", buildTraceGraph(Json)); >From 9fa462d60262c4bed03e95f71d3e17dc2788343f Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 29 Aug 2024 12:27:19 +0000 Subject: [PATCH 4/4] Add comment for not checking verbose --- clang/lib/Parse/Parser.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index d6829045bb4293..bc06145c8d442c 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1257,6 +1257,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclarationOrFunctionDefinition( // "EvaluateAsConstExpr". llvm::TimeTraceScope TimeScope("ParseDeclarationOrFunctionDefinition", [&]() { llvm::TimeTraceMetadata M; + // Parsing events are not as common as instantiations. Add location for them + // without verbose mode check. Actions.getASTContext().getSourceManager().setLocationForTimeTrace( Tok.getLocation(), M); return M; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits