[clang] Use timeTraceAsyncProfilerBegin for Source span (PR #83961)

[Takuto Ikuta via cfe-commits]

https://github.com/atetubou updated 
https://github.com/llvm/llvm-project/pull/83961

>From e61cf0ec2caa594fe915711477083875dc8bf449 Mon Sep 17 00:00:00 2001
From: Takuto Ikuta 
Date: Mon, 4 Mar 2024 17:02:05 +0900
Subject: [PATCH] Use timeTraceAsyncProfilerBegin for Source span

---
 clang/lib/Sema/Sema.cpp | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index cd0c42d5ffbacd..36762b217c9faa 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -135,6 +135,7 @@ namespace sema {
 class SemaPPCallbacks : public PPCallbacks {
   Sema *S = nullptr;
   llvm::SmallVector IncludeStack;
+  llvm::SmallVector ProfilerStack;
 
 public:
   void set(Sema &S) { this->S = &S; }
@@ -153,8 +154,8 @@ class SemaPPCallbacks : public PPCallbacks {
   if (IncludeLoc.isValid()) {
 if (llvm::timeTraceProfilerEnabled()) {
   OptionalFileEntryRef FE = SM.getFileEntryRefForID(SM.getFileID(Loc));
-  llvm::timeTraceProfilerBegin("Source", FE ? FE->getName()
-: StringRef(""));
+  ProfilerStack.push_back(llvm::timeTraceAsyncProfilerBegin(
+  "Source", FE ? FE->getName() : StringRef("")));
 }
 
 IncludeStack.push_back(IncludeLoc);
@@ -166,8 +167,9 @@ class SemaPPCallbacks : public PPCallbacks {
 }
 case ExitFile:
   if (!IncludeStack.empty()) {
-if (llvm::timeTraceProfilerEnabled())
-  llvm::timeTraceProfilerEnd();
+if (llvm::timeTraceProfilerEnabled()) {
+  llvm::timeTraceProfilerEnd(ProfilerStack.pop_back_val());
+}
 
 S->DiagnoseNonDefaultPragmaAlignPack(
 Sema::PragmaAlignPackDiagnoseKind::ChangedStateAtExit,

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

[clang] Use timeTraceAsyncProfilerBegin for Source span (PR #83961)

[Takuto Ikuta via cfe-commits]

atetubou wrote:

@MaskRay could you review this?

https://github.com/llvm/llvm-project/pull/83961
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Use timeTraceAsyncProfilerBegin for Source span (PR #83961)

[Zequan Wu via cfe-commits]

https://github.com/ZequanWu approved this pull request.


https://github.com/llvm/llvm-project/pull/83961
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Use timeTraceAsyncProfilerBegin for Source span (PR #83961)

[Zequan Wu via cfe-commits]

https://github.com/ZequanWu edited 
https://github.com/llvm/llvm-project/pull/83961
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Use timeTraceAsyncProfilerBegin for Source span (PR #83961)

[Zequan Wu via cfe-commits]

@@ -102,23 +104,24 @@ struct llvm::TimeTraceProfiler {
 llvm::get_thread_name(ThreadName);
   }
 
-  void begin(std::string Name, llvm::function_ref Detail) {
-Stack.emplace_back(ClockType::now(), TimePointType(), std::move(Name),
-   Detail());
+  TimeTraceProfilerEntry *begin(std::string Name,
+llvm::function_ref Detail,
+bool AsyncEvent = false) {
+Stack.emplace_back(std::make_unique(
+ClockType::now(), TimePointType(), std::move(Name), Detail(),
+AsyncEvent));
+return Stack.back().get();
   }
 
   void end() {
+TimeTraceProfilerEntry *E = Stack.back().get();

ZequanWu wrote:

Yeah. But usually assertions goes before use. `end()` uses `Stack.back()` which 
assumes stack is non-empty.

https://github.com/llvm/llvm-project/pull/83961
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Use timeTraceAsyncProfilerBegin for Source span (PR #83961)

[Fangrui Song via cfe-commits]

@@ -166,8 +167,9 @@ class SemaPPCallbacks : public PPCallbacks {
 }
 case ExitFile:
   if (!IncludeStack.empty()) {
-if (llvm::timeTraceProfilerEnabled())
-  llvm::timeTraceProfilerEnd();
+if (llvm::timeTraceProfilerEnabled()) {
+  llvm::timeTraceProfilerEnd(ProfilerStack.pop_back_val());

MaskRay wrote:

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

https://github.com/llvm/llvm-project/pull/83961
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Use timeTraceAsyncProfilerBegin for Source span (PR #83961)

[Takuto Ikuta via cfe-commits]

atetubou wrote:

Thank you for your review and merge!

https://github.com/llvm/llvm-project/pull/83961
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits