This is an archive of the discontinued LLVM Phabricator instance.

Fix a time-trace issue of incorrect header hierarchy when a header contains a template function for its last symbol.
AbandonedPublic

Authored by MaggieYi on Apr 4 2023, 4:17 AM.

Details

Summary

HandleEndOfFile is invoked when the lexer hits the end of the current file. This either returns the EOF token or pops a level off the include stack and
keeps going. If it keeps going, clang parses from one header to another header. This results in incorrect header hierarchy in the time trace.

This patch corrects this, as reported by #56554

Fixes: https://github.com/llvm/llvm-project/issues/56554

Diff Detail

Event Timeline

MaggieYi created this revision.Apr 4 2023, 4:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2023, 4:17 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
MaggieYi requested review of this revision.Apr 4 2023, 4:17 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 4 2023, 4:17 AM

A simple test to reproduce the issue:

% cat main.cpp
#include "1.h"
#include "2.h"
int foo();

% cat 1.h
template <typename T> auto Zero() -> T { return T{}; }

%cat 2.h
struct Bla {};

Compile the code with -ftime-trace-granularity=0 -ftime-trace to show the issue:

prospero-clang -ftime-trace-granularity=0 -ftime-trace main.cpp -c -o main.o

Analysis the issue using the above simple example:

When clang parses the first line (#include "1.h") in the main.cpp, a time section entry is created and put on the top of the time-trace stack. Assuming this time section entry is named Source-1.h, which has its name Source and detail 1.h. Then, clang parses the template function Zero defined in the 1.h header file. Clang will create a TimeTraceScope variable in which its name is ParseTemplate and its detail is Zero. A new time section entry named ParseTemplate-Zero is created and put on the top of the time-trace stack. ParseTemplate-Zero has its name ParseTemplate and detail Zero. Now, the top element of the time-trace stack is ParseTemplate-Zero and Source-1.h is under ParseTemplate-Zero.

Please note: since ParseTemplate-Zero is TimeTraceScope type variable. It should be popped out from the time-trace stack once the destructor of TimeTraceScope is called.

Since the template Zero is the last symbol defined in the 1.h header, the LexEndOfFile is called, then LexedFileChanged is called. LexedFileChanged is invoked when the lexer hits the end of the current file. This either returns the EOF token or pops a level off the include stack and keeps going. In our case, it keeps going and notifies the client that we are in a new header file: 2.h. The function of timeTraceProfilerEnd is called to pop the top element out from the time-trace stack and calculate the corresponding compilation time. Source-1.h is expected to pop out from the time-trace stack. However, since ParseTemplate-Zero is still alive and is on the top of the time-trace stack. ParseTemplate-Zero is popped out from the time trace stack instead of Source-1.h, which is wrong. Until now, the top of the time-trace stack is Source-1.h.

After that, a time section entry named Source-2.h is created and put on the top of the time-trace stack. Now, the top element of the time-trace stack is Source-2.h, and then Source-1.h is under Source-2.h. This results in incorrect header hierarchy in the time trace: the 2.h shows up as an inclusion under 1.h.

Currently, two available time profiling APIs cannot deal with this special case. I have modified timeTraceProfilerEnd() to handle the header file specially.

Gentle ping ...

This is probably obsolete now that D148410 has landed.

MaggieYi abandoned this revision.Apr 18 2023, 1:30 AM