MaggieYi (Ying Yi)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 18 2015, 5:38 AM (152 w, 2 h)

Recent Activity

Nov 28 2017

MaggieYi updated the diff for D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.

The previous patch doesn’t support the sequential commands, for example “mkdir existing_folder || true”. This caused sanitizer-windows test failed. The patch has been updated to support the sequential commands.

Nov 28 2017, 3:19 AM

Nov 23 2017

MaggieYi reopened D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.

Reverted rL318911 since it broke the sanitizer-windows.

Nov 23 2017, 5:27 AM

Nov 20 2017

MaggieYi updated the diff for D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.

The test of MC/AsmParser/preserve-comments-crlf.s failed since it uses the unsupported diff option --strip-trailing-cr. I updated the internal diff shell command to support this option.

Nov 20 2017, 4:23 PM

Nov 10 2017

MaggieYi updated the diff for D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.

Optimised the patch by using python ‘getopt’ module.

Nov 10 2017, 2:08 PM

Nov 9 2017

MaggieYi updated the diff for D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.

Updated the builtin diff shell to support ‘-u’, ‘-b’ and ‘-w’ options. Now, all llvm, and clang tests passed with both Python 2 and Python 3.

Nov 9 2017, 1:59 PM

Nov 7 2017

MaggieYi added a comment to D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.
  1. I don't see any tests for the case where the commands *succeed*. Only for the case where they fail. Shouldn't we have some tests that verify the commands actually work?
Nov 7 2017, 6:30 AM
MaggieYi updated the diff for D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.

The patch updated the test (valid-shell.txt) where lit internal shell commands succeeded.

Nov 7 2017, 6:14 AM

Nov 6 2017

MaggieYi added a comment to D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.

Update: I actually do see where you're testing that they work, but I'm not convinced that the tests are sufficient. For several reasons.

  1. it doesn't test that the builtin command is actually run versus the OS command. It only tests that whatever command is run doesn't fail.
  2. It doesn't test that the result of the command is as expected, it only tests that it doesn't return an error. For a somewhat unrealistic illustration of this, imagine that all the commands just returned success without actually doing anything. The test as it is written now would pass.
Nov 6 2017, 5:34 AM
MaggieYi added a comment to D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.
  1. Now that we have upwards of 6 builtin commands, I'm starting to think they should be separated into their own file. For example, lit/test/ShellBuiltins.py We could test them via unittests (e.g. see how BooleanExpression.py implements its unit tests).

    What do you think?
Nov 6 2017, 5:33 AM

Nov 2 2017

MaggieYi created D39567: [lit] Implement non-pipelined ‘mkdir’, ‘diff’ and ‘rm’ commands internally.
Nov 2 2017, 2:10 PM

Oct 18 2016

MaggieYi added a comment to D25086: [llvm-cov] Add support for loading coverage from multiple objects.
In D25086#572183, @vsk wrote:

@MaggieYi ping, do you have the bandwidth to update this patch to use the new CoverageMapping::load API? If not, I can take a shot at it.

Oct 18 2016, 3:43 AM

Sep 30 2016

MaggieYi retitled D25086: [llvm-cov] Add support for loading coverage from multiple objects from to [llvm-cov] Allow llvm-cov to report the coverage counters across multiple elf files..
Sep 30 2016, 2:33 AM

Sep 27 2016

MaggieYi abandoned D24687: [llvm-cov] Add a project coverage summary above the index page..
Sep 27 2016, 3:44 AM

Sep 26 2016

MaggieYi abandoned D24586: [llvm-cov] Add filter and sort table functions to the index page..
Sep 26 2016, 12:25 AM

Sep 20 2016

MaggieYi updated the diff for D24687: [llvm-cov] Add a project coverage summary above the index page..
In D24687#546833, @vsk wrote:

I'd prefer to just tackle this issue in this patch. IMO it would be better to drop (or at least defer) the other changes.

Ok, I have updated the patch following your advice.

Sep 20 2016, 1:52 PM

Sep 19 2016

MaggieYi abandoned D24367: [llvm-cov] Provided easy navigation to find the previous and next report..

I agree with you and abandon this patch.

Sep 19 2016, 4:28 AM

Sep 16 2016

MaggieYi retitled D24687: [llvm-cov] Add a project coverage summary above the index page. from to [llvm-cov] Add a project coverage summary above the index page..
Sep 16 2016, 3:12 PM

Sep 14 2016

MaggieYi retitled D24586: [llvm-cov] Add filter and sort table functions to the index page. from to [llvm-cov] Add filter and sort table functions to the index page..
Sep 14 2016, 2:45 PM

Sep 13 2016

MaggieYi added inline comments to D24367: [llvm-cov] Provided easy navigation to find the previous and next report..
Sep 13 2016, 7:42 AM
MaggieYi updated the diff for D24367: [llvm-cov] Provided easy navigation to find the previous and next report..
In D24367#539047, @vsk wrote:

On a low level, I'm concerned about the unnecessary quadratic behavior in this patch. If "N = |SourceFiles|", then renderPageNavigation() does O(N) work and it's called O(N) times. Given that llvm+clang has over 2,400 source files, this could actually be noticeable. We already get user reports about llvm-cov being slow...

Sep 13 2016, 7:40 AM

Sep 12 2016

MaggieYi retitled D24457: [llvm-cov] - Included footer "Generated by llvm-cov -- llvm version <version number>" in the coverage report. from to [llvm-cov] - Included footer "Generated by llvm-cov -- llvm version <version number>" in the coverage report..
Sep 12 2016, 8:15 AM

Sep 9 2016

MaggieYi added reviewers for D24367: [llvm-cov] Provided easy navigation to find the previous and next report.: davidxl, bogner.
Sep 9 2016, 12:33 AM

Sep 8 2016

MaggieYi retitled D24367: [llvm-cov] Provided easy navigation to find the previous and next report. from to [llvm-cov] Provided easy navigation to find the previous and next report..
Sep 8 2016, 4:38 PM

Sep 5 2016

MaggieYi retitled D24241: [llvm-cov] Add the project summary to the text coverage report for each source file. from to [llvm-cov] Add the project summary to the text coverage report for each source file..
Sep 5 2016, 1:21 PM

Sep 2 2016

MaggieYi updated the diff for D23277: [llvm-cov] Add the "Goto first zero count" feature..

Thanks for your pseudocode . Two issues have been fixed.

Sep 2 2016, 2:00 PM

Sep 1 2016

MaggieYi updated the diff for D23277: [llvm-cov] Add the "Goto first zero count" feature..

Thanks for your explanation. I have updated the patch following your advice.

Sep 1 2016, 4:39 AM

Aug 30 2016

MaggieYi added a comment to D23277: [llvm-cov] Add the "Goto first zero count" feature..
In D23277#526594, @vsk wrote:

Thanks for splitting things up :).

I'm not sure that the javascript bit is needed. It looks like a way to remove/disable a link, but I don't see any issue with leaving the link active.

Aug 30 2016, 4:09 PM
MaggieYi updated the diff for D23277: [llvm-cov] Add the "Goto first zero count" feature..

Reused the existing native_separator.c test to check fully covered source file case.

Aug 30 2016, 3:58 PM

Aug 29 2016

MaggieYi updated the diff for D23922: [llvm-cov] Use the native path in the coverage report..

Thanks, the test has been added.

Aug 29 2016, 3:34 PM

Aug 28 2016

MaggieYi added inline comments to D23922: [llvm-cov] Use the native path in the coverage report..
Aug 28 2016, 12:35 PM
MaggieYi updated the diff for D23922: [llvm-cov] Use the native path in the coverage report..

Thanks Vedant, the patch has been updated following your comments.

Aug 28 2016, 12:25 PM

Aug 26 2016

MaggieYi retitled D23922: [llvm-cov] Use the native path in the coverage report. from to [llvm-cov] Use the native path in the coverage report..
Aug 26 2016, 8:19 AM

Aug 24 2016

MaggieYi updated the diff for D23277: [llvm-cov] Add the "Goto first zero count" feature..

Updating the patch to support the "Goto first zero count" feature.

Aug 24 2016, 2:02 PM

Aug 23 2016

MaggieYi added inline comments to D23345: [llvm-cov] Add the project summary to each source file coverage report..
Aug 23 2016, 4:35 PM
MaggieYi updated the diff for D23345: [llvm-cov] Add the project summary to each source file coverage report..

I have fixed the issues, Thanks.

Aug 23 2016, 4:35 PM
MaggieYi added inline comments to D23345: [llvm-cov] Add the project summary to each source file coverage report..
Aug 23 2016, 12:41 PM
MaggieYi updated the diff for D23345: [llvm-cov] Add the project summary to each source file coverage report..

Hi Vedant,

Aug 23 2016, 12:40 PM

Aug 16 2016

MaggieYi updated subscribers of D23486: [llvm-cov] Add coverage summary information to the HTML index page..
Aug 16 2016, 4:41 AM

Aug 11 2016

MaggieYi added a comment to D23345: [llvm-cov] Add the project summary to each source file coverage report..

Many thanks for your comments. As I will be busy on other work in next several days, I will update the patch following your comments once I get those on track. Thanks for the patience.

Aug 11 2016, 8:28 AM

Aug 10 2016

MaggieYi updated the diff for D23277: [llvm-cov] Add the "Goto first zero count" feature..

I have put the change of adding the project summary into Phabricator (https://reviews.llvm.org/D23345).

Aug 10 2016, 5:20 AM
MaggieYi retitled D23345: [llvm-cov] Add the project summary to each source file coverage report. from to [llvm-cov] Add the project summary to each source file coverage report..
Aug 10 2016, 1:27 AM

Aug 9 2016

MaggieYi added a comment to D23281: [LLVMCov] Swapped the line and count columns..

Hello Vedant,

Aug 9 2016, 1:15 PM
MaggieYi added a comment to D23281: [LLVMCov] Swapped the line and count columns..
In D23281#510048, @vsk wrote:

I think it would be fine to commit the two patches separately but simultaneously. The bots may go red temporarily, but that won't be alarming if you mention that a follow-up commit in compiler-rt is needed in your commit message.

Do you have a linux machine to test the changes to test/profile/Linux/coverage_*? If not, I can help with that.

Aug 9 2016, 11:37 AM
MaggieYi updated the diff for D23281: [LLVMCov] Swapped the line and count columns..

Fix a linux test.

Aug 9 2016, 7:15 AM
MaggieYi updated the diff for D23281: [LLVMCov] Swapped the line and count columns..

Thanks Vedant.

Aug 9 2016, 6:54 AM

Aug 8 2016

MaggieYi added a comment to D23277: [llvm-cov] Add the "Goto first zero count" feature..
In D23277#508994, @vsk wrote:

On a high-level, the changes you've made look good. However, I think that this
patch needs to be split up into 3 smaller patches for each functional change
you listed in your summary. That would make it easier for me to review it, and
would make the commit history easier to read through.

Aug 8 2016, 2:08 PM
MaggieYi retitled D23281: [LLVMCov] Swapped the line and count columns. from to [LLVMCov] Swapped the line and count columns..
Aug 8 2016, 2:01 PM
MaggieYi retitled D23277: [llvm-cov] Add the "Goto first zero count" feature. from to Improves the HTML report for the source file..
Aug 8 2016, 11:38 AM

Aug 3 2016

MaggieYi added a comment to D23087: [LLVM-COV] Replace tabs to 2-space indentation in the HTML coverage report..

Following Vedant's comments, all issues have been fixed.

Aug 3 2016, 6:33 AM
MaggieYi updated the diff for D23087: [LLVM-COV] Replace tabs to 2-space indentation in the HTML coverage report..

Thanks Vedant, I have updated the patch following your comments.

Aug 3 2016, 6:31 AM

Aug 2 2016

MaggieYi retitled D23087: [LLVM-COV] Replace tabs to 2-space indentation in the HTML coverage report. from to [LLVM-COV] Replace tabs to 2-space indentation in the HTML coverage report..
Aug 2 2016, 3:26 PM

Jul 26 2016

MaggieYi added a comment to D22692: [utils] Test the code coverage regression-checking script.
In D22692#495195, @vsk wrote:

Could you share more details about how you ran the test? I'm having a hard time understanding the failure because LLVM_SRC_ROOT is set in test/lit.cfg. That environment variable should be set on Windows builders if the test is run with llvm-lit test/tools/llvm-cov/report.cpp.

Jul 26 2016, 11:49 AM

Jul 24 2016

MaggieYi added a comment to D22692: [utils] Test the code coverage regression-checking script.

Thanks for adding the test.

Jul 24 2016, 1:49 AM

Jul 21 2016

MaggieYi added a comment to D22569: [LLVM-COV] Add the coverage of lines in the summary report..

The test in test/tools/llvm-cov/prevent_false_instantiations.h has been updated.

Jul 21 2016, 7:17 AM
MaggieYi updated the diff for D22569: [LLVM-COV] Add the coverage of lines in the summary report..

Thanks Vedant. I have updated the patch following your comments.

Jul 21 2016, 7:13 AM
MaggieYi retitled D22621: [llvm-cov] - Improve llvm-cov error message from to [llvm-cov] - Improve llvm-cov error message.
Jul 21 2016, 3:27 AM

Jul 20 2016

MaggieYi updated D22569: [LLVM-COV] Add the coverage of lines in the summary report..
Jul 20 2016, 7:12 AM
MaggieYi retitled D22569: [LLVM-COV] Add the coverage of lines in the summary report. from to [LLVM-COV] Add the coverage of lines in the summary report..
Jul 20 2016, 7:09 AM

Jul 6 2016

MaggieYi added a comment to D18278: llvm-cov HTML Generation.
In D18278#474741, @vsk wrote:

Thanks for testing it out!

It seems that all the pending issues we are discussing actually relate to CoveragePrinter::getOutputPath, which is not a part of this patch. We can fix them before or after this patch is committed, and test the changes using "-format text -output-dir ...".

Given that this is a large change, I'd like to make sure it addresses Justin's concerns and that it has an explicit lgtm before moving forward.

Jul 6 2016, 12:36 AM

Jul 4 2016

MaggieYi added a comment to D18278: llvm-cov HTML Generation.

There are two issues with the current patch on Windows, I am happy to patch these later if you want.

Jul 4 2016, 1:46 AM

Mar 21 2016

MaggieYi added a comment to D18278: llvm-cov HTML Generation.

Thanks Harlan, LGTM.

Mar 21 2016, 1:33 AM

Mar 2 2016

MaggieYi added a comment to D17676: Add some minimal portability code paths for PS4..

Thanks Sean, the patch looks good to me.

Mar 2 2016, 5:22 AM

Dec 17 2015

MaggieYi added a comment to D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4.

Thanks Paul for your help.

Dec 17 2015, 9:02 AM

Dec 16 2015

MaggieYi added a comment to D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4.

Could someone commit it for me please (as I do not have commit access)? Thanks

Dec 16 2015, 6:09 AM

Dec 15 2015

MaggieYi updated the diff for D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4.

Thanks, I have updated the test following your comments.

Dec 15 2015, 10:16 AM
MaggieYi added a comment to D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4.

My patch changes 6 compiler flags, which are –coverage, -fprofile-arcs, -fprofile-generate, -fprofile-generate=, -fprofile-instr-generate, -fprofile-instr-generate=. I would like to keep line 7-10 in order to verify the change to using “hasFlag” instead of “hasArg”. For the other switches, I could simplify the tests to only check “–fx” and “–fno-x” in order to cut down on test proliferation.

Dec 15 2015, 7:44 AM

Dec 14 2015

MaggieYi updated the diff for D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4.

Following Vedant's comments, two test issues have been fixed.

Dec 14 2015, 6:35 AM
MaggieYi added a comment to D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4.

Thanks for reviewing the patch. I try to answer your questions, could you please let me know if this makes sense to you?

Dec 14 2015, 6:30 AM

Dec 11 2015

MaggieYi added a comment to D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4.

Ping

Dec 11 2015, 12:40 AM

Dec 4 2015

MaggieYi added a reviewer for D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4: FlameTop.
Dec 4 2015, 6:21 AM
MaggieYi retitled D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4 from to [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4.
Dec 4 2015, 2:24 AM

Aug 27 2015

MaggieYi added a comment to D12163: [Patch] [Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph" (PR 24184).

Thanks for helping me review and submit the patch.

Aug 27 2015, 1:20 PM
MaggieYi added a comment to D12163: [Patch] [Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph" (PR 24184).

Ping

Aug 27 2015, 4:23 AM

Aug 20 2015

MaggieYi updated the diff for D12163: [Patch] [Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph" (PR 24184).

Hi Anna,

Aug 20 2015, 2:22 AM

Aug 19 2015

MaggieYi retitled D12163: [Patch] [Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph" (PR 24184) from to [Patch] [Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph" (PR 24184).
Aug 19 2015, 10:45 AM