Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

dongjunduo (dongjunduo)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 8 2022, 7:46 AM (81 w, 3 d)

Recent Activity

Oct 25 2022

dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

Hey @dongjunduo,
Are you still interested in finishing this? If not I was thinking that I would pick it up, so I can work on the follow ups.

Oct 25 2022, 5:18 PM · Restricted Project, Restricted Project

Sep 22 2022

dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

As discussed with @jamieschmeiser on D133662, I have left suggestions regarding the approach I took for handling -o and passing the option to the jobs.

I'm really happy to see this area getting attention, and I'm sorry for the confusion I must have caused.

Sep 22 2022, 8:48 AM · Restricted Project, Restricted Project

Sep 13 2022

dongjunduo added inline comments to D131469: [Clang] change default storing path of `-ftime-trace`.
Sep 13 2022, 11:47 PM · Restricted Project, Restricted Project
dongjunduo added a reviewer for D133662: [Clang] Change -ftime-trace storing path and support multiple compilation jobs: dongjunduo.
Sep 13 2022, 6:16 AM · Restricted Project, Restricted Project
dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

@MaskRay ping

Sep 13 2022, 6:10 AM · Restricted Project, Restricted Project

Sep 12 2022

dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

I appreciate the detailed summary. It has sufficient information but I think it can be rephased to be conciser.
I request changes for the verbosity and the possibly functionality issue.

We can use -ftime-trace or -ftime-trace=<path> to switch on the TimeProfiler.

"We can use" can be omitted => "-ftime-trace or -ftime-trace=<path> enables ..."

The source files should be compiled into object files (.o) by clang, and then linked into executable file by the linker.

This is basic knowledge and can be removed.

"-ftime-trace or -ftime-trace=<path> enables ..."

Where to store for now?

If we want a detailed description, the whole paragraph is really something which should go to https://clang.llvm.org/docs/UsersManual.html
Then, this patch summary (commit message) can just refer to it.


I'd delete everything and keep just

-ftime-trace and -ftime-trace=<path> enable the TimeProfiler.
When Clang does compile and link actions in one command and -ftime-trace is used, the JSON file currently goes to a temporary directory, e.g.

$ clang++ -ftime-trace -o main.out /demo/main.cpp
$ ls .
main.out
$ ls /tmp/
main-[random-string].json

The JSON file location is undesired. This patch changes the location based on the specified ...


What if an input file and the output is in directories, e.g.

% myclang -ftime-trace a/a.c -o b/a; ls *.json
a-a7cffa.json

Does the patch achieve what it intends to do?

Sep 12 2022, 5:18 AM · Restricted Project, Restricted Project
dongjunduo updated the summary of D131469: [Clang] change default storing path of `-ftime-trace`.
Sep 12 2022, 5:15 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

fix related nits

Sep 12 2022, 4:50 AM · Restricted Project, Restricted Project

Sep 11 2022

dongjunduo committed rG6975ab71260c: [Clang] Reimplement time tracing of NewPassManager by PassInstrumentation… (authored by dongjunduo).
[Clang] Reimplement time tracing of NewPassManager by PassInstrumentation…
Sep 11 2022, 7:56 AM · Restricted Project, Restricted Project
dongjunduo closed D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Sep 11 2022, 7:56 AM · Restricted Project, Restricted Project

Sep 7 2022

dongjunduo updated the diff for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

Restyle codes and comments

Sep 7 2022, 7:04 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

fix windows path-check error

Sep 7 2022, 6:47 AM · Restricted Project, Restricted Project

Sep 6 2022

dongjunduo updated the diff for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

Add more comments

Sep 6 2022, 7:13 PM · Restricted Project, Restricted Project
dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

@dyung @steven_wu A newer diff has been submitted just now.

Sep 6 2022, 8:13 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Rewrite test by checking output of -###

Sep 6 2022, 8:08 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

Remove the useless code

Sep 6 2022, 6:44 AM · Restricted Project, Restricted Project
dongjunduo reopened D131469: [Clang] change default storing path of `-ftime-trace`.
Sep 6 2022, 6:40 AM · Restricted Project, Restricted Project

Sep 5 2022

dongjunduo updated the diff for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

Restyle codes

Sep 5 2022, 6:02 PM · Restricted Project, Restricted Project
dongjunduo added inline comments to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Sep 5 2022, 6:19 AM · Restricted Project, Restricted Project
dongjunduo added inline comments to D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Sep 5 2022, 6:17 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

Restyle the callback registration and related comment

Sep 5 2022, 6:10 AM · Restricted Project, Restricted Project

Sep 3 2022

dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

These related commits have been reverted temporarily.

Sep 3 2022, 1:42 AM · Restricted Project, Restricted Project
dongjunduo added a reverting change for rG38941da066a7: [Clang] change default storing path of `-ftime-trace`: rG5e4f69edbc1e: Revert "[Clang] change default storing path of `-ftime-trace`".
Sep 3 2022, 1:39 AM · Restricted Project, Restricted Project
dongjunduo committed rG5e4f69edbc1e: Revert "[Clang] change default storing path of `-ftime-trace`" (authored by dongjunduo).
Revert "[Clang] change default storing path of `-ftime-trace`"
Sep 3 2022, 1:39 AM · Restricted Project, Restricted Project
dongjunduo added a reverting change for rG39221ad55752: [driver][clang] remove the check-time-trace test on the platform…: rG4ff2250a22ed: Revert "[driver][clang] remove the check-time-trace test on the platform….
Sep 3 2022, 1:39 AM · Restricted Project, Restricted Project
dongjunduo committed rG4ff2250a22ed: Revert "[driver][clang] remove the check-time-trace test on the platform… (authored by dongjunduo).
Revert "[driver][clang] remove the check-time-trace test on the platform…
Sep 3 2022, 1:39 AM · Restricted Project, Restricted Project
dongjunduo added a reverting change for D131469: [Clang] change default storing path of `-ftime-trace`: rG5e4f69edbc1e: Revert "[Clang] change default storing path of `-ftime-trace`".
Sep 3 2022, 1:39 AM · Restricted Project, Restricted Project

Sep 2 2022

dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

The test you added is failing on the PS4 linux bot because the PS4 platform requires an external linker that isn't present. Is linking necessary for your test? Or can -S or even -c work instead to accomplish what you are trying to test?

Yeah the new test case is designed to test the compiling jobs with a linking stage.

Are there any options or measures to avoid this test running on the PS4?

You could mark it as XFAIL: ps4

Your change also seems to have possibly the same issue when run on our PS5 Windows bot:
https://lab.llvm.org/buildbot/#/builders/216/builds/9260

$ ":" "RUN: at line 2"
$ "z:\test\build\bin\clang.exe" "--driver-mode=g++" "-ftime-trace" "-ftime-trace-granularity=0" "-o" "Z:\test\build\tools\clang\test\Driver\Output/exe/check-time-trace" "Z:\test\llvm-project\clang\test\Driver\check-time-trace.cpp"
# command stderr:
clang: error: unable to execute command: program not executable
clang: error: linker command failed with exit code 1 (use -v to see invocation)

How about "UNSUPPORTED: ps4, ps5"

That would likely work I think.

Sep 2 2022, 8:31 PM · Restricted Project, Restricted Project
dongjunduo committed rG39221ad55752: [driver][clang] remove the check-time-trace test on the platform… (authored by dongjunduo).
[driver][clang] remove the check-time-trace test on the platform…
Sep 2 2022, 8:28 PM · Restricted Project, Restricted Project
dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

The test you added is failing on the PS4 linux bot because the PS4 platform requires an external linker that isn't present. Is linking necessary for your test? Or can -S or even -c work instead to accomplish what you are trying to test?

Yeah the new test case is designed to test the compiling jobs with a linking stage.

Are there any options or measures to avoid this test running on the PS4?

You could mark it as XFAIL: ps4

Your change also seems to have possibly the same issue when run on our PS5 Windows bot:
https://lab.llvm.org/buildbot/#/builders/216/builds/9260

$ ":" "RUN: at line 2"
$ "z:\test\build\bin\clang.exe" "--driver-mode=g++" "-ftime-trace" "-ftime-trace-granularity=0" "-o" "Z:\test\build\tools\clang\test\Driver\Output/exe/check-time-trace" "Z:\test\llvm-project\clang\test\Driver\check-time-trace.cpp"
# command stderr:
clang: error: unable to execute command: program not executable
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Sep 2 2022, 7:34 PM · Restricted Project, Restricted Project
dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

The test you added is failing on the PS4 linux bot because the PS4 platform requires an external linker that isn't present. Is linking necessary for your test? Or can -S or even -c work instead to accomplish what you are trying to test?

Sep 2 2022, 7:16 PM · Restricted Project, Restricted Project
dongjunduo committed rG38941da066a7: [Clang] change default storing path of `-ftime-trace` (authored by dongjunduo).
[Clang] change default storing path of `-ftime-trace`
Sep 2 2022, 6:50 PM · Restricted Project, Restricted Project
dongjunduo closed D131469: [Clang] change default storing path of `-ftime-trace`.
Sep 2 2022, 6:49 PM · Restricted Project, Restricted Project
dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

For now, the time-trace file's name is corresponding to the output file's name ([demo].o => [demo].json).

Sep 2 2022, 7:21 AM · Restricted Project, Restricted Project

Aug 30 2022

dongjunduo retitled D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework from [WIP] [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework to [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 30 2022, 5:39 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

Adjust the location of TimeProfiler's start and end functions

Aug 30 2022, 5:28 AM · Restricted Project, Restricted Project

Aug 29 2022

dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Rewrite comments and commit log

Aug 29 2022, 8:42 PM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Replace -ftime-trace with -ftime-trace=<infered_path> also

Aug 29 2022, 8:37 PM · Restricted Project, Restricted Project
dongjunduo added inline comments to D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 29 2022, 7:30 PM · Restricted Project, Restricted Project
dongjunduo added inline comments to D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 29 2022, 7:15 PM · Restricted Project, Restricted Project
dongjunduo updated the summary of D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 29 2022, 7:25 AM · Restricted Project, Restricted Project
dongjunduo added inline comments to D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 29 2022, 1:40 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Restyle variables' name and comments

Aug 29 2022, 1:39 AM · Restricted Project, Restricted Project

Aug 22 2022

dongjunduo updated the diff for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.

Move class PassTimeProfiling to StandardInstrumentation.cpp

Aug 22 2022, 7:51 AM · Restricted Project, Restricted Project
dongjunduo retitled D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework from [Clang] Reimplement time tracing of NewPassManager by PassInstrumentation framework to [WIP] [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 22 2022, 7:47 AM · Restricted Project, Restricted Project
dongjunduo added inline comments to D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 22 2022, 7:39 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Restyle code

Aug 22 2022, 7:35 AM · Restricted Project, Restricted Project
dongjunduo added inline comments to D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 22 2022, 3:18 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Restyle code

Aug 22 2022, 3:17 AM · Restricted Project, Restricted Project

Aug 16 2022

dongjunduo added reviewers for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework: fedor.sergeev, anton-afanasyev.
Aug 16 2022, 7:09 AM · Restricted Project, Restricted Project
dongjunduo updated the summary of D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 16 2022, 6:22 AM · Restricted Project, Restricted Project
dongjunduo updated the summary of D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 16 2022, 6:14 AM · Restricted Project, Restricted Project
dongjunduo updated the summary of D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 16 2022, 6:13 AM · Restricted Project, Restricted Project
dongjunduo added a reviewer for D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework: amonshiz.
Aug 16 2022, 6:13 AM · Restricted Project, Restricted Project
dongjunduo updated the summary of D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 16 2022, 6:00 AM · Restricted Project, Restricted Project
dongjunduo updated the summary of D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 16 2022, 5:56 AM · Restricted Project, Restricted Project
dongjunduo requested review of D131960: [IR] Reimplement time tracing of NewPassManager by PassInstrumentation framework.
Aug 16 2022, 5:55 AM · Restricted Project, Restricted Project

Aug 11 2022

dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Add assert messages

Aug 11 2022, 7:31 PM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Add necessary asserts

Aug 11 2022, 7:13 PM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

format cc1_main.cpp

Aug 11 2022, 8:44 AM · Restricted Project, Restricted Project
dongjunduo added inline comments to D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 11 2022, 8:10 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Restyle codes

Aug 11 2022, 8:07 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

format code

Aug 11 2022, 7:38 AM · Restricted Project, Restricted Project
dongjunduo added a comment to D131469: [Clang] change default storing path of `-ftime-trace`.

You should not have debugging information in code that is up for review. If this is debugging information that you plan to leave in for future purposes (which I doubt is the case here), you need to protect it so that it isn't active unless some option is set. For example, see LLVM_DEBUG code that lives in various opt routines.

Aug 11 2022, 6:52 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

fix stringRef bug

Aug 11 2022, 6:48 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Add more debug info

Aug 11 2022, 4:55 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

Add more debug info

Aug 11 2022, 3:40 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D131469: [Clang] change default storing path of `-ftime-trace`.

[Clang] add -### for debug

Aug 11 2022, 2:48 AM · Restricted Project, Restricted Project

Aug 9 2022

dongjunduo added reviewers for D131469: [Clang] change default storing path of `-ftime-trace`: jamieschmeiser, Whitney.
Aug 9 2022, 12:50 AM · Restricted Project, Restricted Project
dongjunduo requested review of D131469: [Clang] change default storing path of `-ftime-trace`.
Aug 9 2022, 12:49 AM · Restricted Project, Restricted Project

Jul 15 2022

dongjunduo committed rGf5d9de8cc330: [Clang] Add a new clang option "-ftime-trace=<value>" (authored by dongjunduo).
[Clang] Add a new clang option "-ftime-trace=<value>"
Jul 15 2022, 8:56 AM · Restricted Project, Restricted Project
dongjunduo closed D128048: Add a new clang option "-ftime-trace=<value>".
Jul 15 2022, 8:56 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] fix mkdir error in ftime-trace=<value> test

Jul 15 2022, 5:19 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] restyle code

Jul 15 2022, 12:28 AM · Restricted Project, Restricted Project

Jun 30 2022

dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] format the source file

Jun 30 2022, 6:59 PM · Restricted Project, Restricted Project
dongjunduo retitled D128048: Add a new clang option "-ftime-trace=<value>" from Add a new clang option "-ftime-trace" to Add a new clang option "-ftime-trace=<value>".
Jun 30 2022, 6:53 PM · Restricted Project, Restricted Project
dongjunduo retitled D128048: Add a new clang option "-ftime-trace=<value>" from Add a new clang option "-ftime-trace-path" to Add a new clang option "-ftime-trace".
Jun 30 2022, 6:51 PM · Restricted Project, Restricted Project
dongjunduo added inline comments to D128048: Add a new clang option "-ftime-trace=<value>".
Jun 30 2022, 6:47 PM · Restricted Project, Restricted Project
dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] change test cases to different directory

Jun 30 2022, 6:44 PM · Restricted Project, Restricted Project
dongjunduo added a comment to D128048: Add a new clang option "-ftime-trace=<value>".

Hi @jamieschmeiser @Whitney @MaskRay, I have changed "-ftime-trace-path" to "-ftime-trace". It is a well-spelling option name.

Jun 30 2022, 5:59 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] change "-ftime-trace-path" to "-ftime-trace"

Jun 30 2022, 5:49 AM · Restricted Project, Restricted Project

Jun 28 2022

dongjunduo added inline comments to D128048: Add a new clang option "-ftime-trace=<value>".
Jun 28 2022, 6:42 AM · Restricted Project, Restricted Project

Jun 24 2022

dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] rewrite test case of "-ftime-trace-path"

Jun 24 2022, 1:04 AM · Restricted Project, Restricted Project

Jun 23 2022

dongjunduo updated the summary of D128048: Add a new clang option "-ftime-trace=<value>".
Jun 23 2022, 7:30 AM · Restricted Project, Restricted Project
dongjunduo added a comment to D128048: Add a new clang option "-ftime-trace=<value>".

I have changed the approach from directory-store to path-store, so that the user can specify the aim path to store the time trace json file.
If "-ftime-trace-path" is not specified, it will follow the default behavior.

Jun 23 2022, 7:28 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] change directory-store to path-store

Jun 23 2022, 7:22 AM · Restricted Project, Restricted Project

Jun 22 2022

dongjunduo added a comment to D128048: Add a new clang option "-ftime-trace=<value>".

Can you please use git rebase -i to collapse all the changes into a single change? If this isn't done, it is difficult to know what is being reviewed as the changes only show the differences since your last revision, not all of the changes.

Jun 22 2022, 6:46 AM · Restricted Project, Restricted Project

Jun 21 2022

dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] update help text of "ftime-trace-path"

Jun 21 2022, 12:53 AM · Restricted Project, Restricted Project

Jun 20 2022

dongjunduo added a comment to D128048: Add a new clang option "-ftime-trace=<value>".

Can you please add some test cases?

Jun 20 2022, 2:17 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] change unclear help text pf "-ftime-trace-path"

Jun 20 2022, 2:15 AM · Restricted Project, Restricted Project
dongjunduo updated the diff for D128048: Add a new clang option "-ftime-trace=<value>".

[Clang] Restore the old behaviors when "-ftime-trace-path" is not specified

Jun 20 2022, 1:33 AM · Restricted Project, Restricted Project

Jun 17 2022

dongjunduo added reviewers for D128048: Add a new clang option "-ftime-trace=<value>": Whitney, jamieschmeiser.
Jun 17 2022, 6:10 AM · Restricted Project, Restricted Project
dongjunduo requested review of D128048: Add a new clang option "-ftime-trace=<value>".
Jun 17 2022, 6:07 AM · Restricted Project, Restricted Project