[clang] [clang] Output location info in separate fields for -ftime-trace (PR #106277)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 29 05:27:32 PDT 2024
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 <usx at 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 <usx at 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 <usx at 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 <usx at 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;
More information about the cfe-commits
mailing list