Page MenuHomePhabricator

sajjadm (Sajjad Mirza)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 15 2019, 11:53 AM (72 w, 5 d)

Recent Activity

Nov 15 2019

sajjadm committed rG97c742e6b74e: [llvm-cov] Fix illegal cast from uint64_t to int64_t (authored by sajjadm).
[llvm-cov] Fix illegal cast from uint64_t to int64_t
Nov 15 2019, 6:19 PM
sajjadm closed D70200: [llvm-cov] Fix illegal cast from uint64_t to int64_t.
Nov 15 2019, 6:19 PM · Restricted Project

Nov 14 2019

sajjadm retitled D70200: [llvm-cov] Fix illegal cast from uint64_t to int64_t from Fix illegal cast from uint64_t to int64_t to [llvm-cov] Fix illegal cast from uint64_t to int64_t.
Nov 14 2019, 3:31 PM · Restricted Project
sajjadm added inline comments to D70200: [llvm-cov] Fix illegal cast from uint64_t to int64_t.
Nov 14 2019, 2:45 PM · Restricted Project
sajjadm updated the diff for D70200: [llvm-cov] Fix illegal cast from uint64_t to int64_t.

Changed to using std::min

Nov 14 2019, 2:45 PM · Restricted Project
sajjadm added a comment to D70200: [llvm-cov] Fix illegal cast from uint64_t to int64_t.

Ha, nice! LGTM. Is there any test you could easily update or add to reproduce this corner case?

Nov 14 2019, 2:26 PM · Restricted Project

Nov 13 2019

sajjadm updated the diff for D70200: [llvm-cov] Fix illegal cast from uint64_t to int64_t.
  • Added comment to clamp function.
Nov 13 2019, 4:40 PM · Restricted Project
sajjadm added a comment to D70200: [llvm-cov] Fix illegal cast from uint64_t to int64_t.

Discovered this because we were getting negative numbers in some of our coverage data when we had counter overflows.

Nov 13 2019, 12:20 PM · Restricted Project
sajjadm created D70200: [llvm-cov] Fix illegal cast from uint64_t to int64_t.
Nov 13 2019, 12:18 PM · Restricted Project

Oct 2 2019

sajjadm added a comment to D68351: [profile] Add a mode to continuously sync counter updates to a file.
In D68351#1691969, @vsk wrote:
In D68351#1691915, @vsk wrote:

Not sure it belongs in this CL specifically, but would it be possible to later add a mode that defers the initialization of the mmap'd file until specifically requested by the process?

This sounds tricky to me, as the initialization requires some work to be done in each copy of the profile runtime linked into the process.

Could you clarify whether you're referring to processes which use the profiling runtime's static initializer, or not? My understanding is that for processes which do use the runtime's static initializer, it's always desirable to set up the mmap'd file in that initializer. If that's not true, I'd appreciate a counterexample. For processes that don't use the runtime's static initializer, the situation (at Apple, at least) is typically that it's a kernel extension or bare-metal piece of firmware, and the continuous mode isn't really applicable anyway.

We do use the static initializer, but in our use case the process doesn't necessarily know where to write coverage data until some other application-specific code has run.

I see, thanks for explaining.

Ideally, I would like to be able to have the instrumented process receive a shared memory buffer from another process, do continuous profile sync into that memory region, and not care whether it is file-backed or anonymous mapping. So it would be great if application code could give the runtime some sort of handle to that object [1] and tell the runtime to use that space for profiling.

I see. Is the lack of support for value profiling a concern, or would this just be for code coverage?

Oct 2 2019, 1:47 PM · Restricted Project, Restricted Project, Restricted Project
sajjadm added a comment to D68351: [profile] Add a mode to continuously sync counter updates to a file.
In D68351#1691915, @vsk wrote:

Not sure it belongs in this CL specifically, but would it be possible to later add a mode that defers the initialization of the mmap'd file until specifically requested by the process?

This sounds tricky to me, as the initialization requires some work to be done in each copy of the profile runtime linked into the process.

Could you clarify whether you're referring to processes which use the profiling runtime's static initializer, or not? My understanding is that for processes which do use the runtime's static initializer, it's always desirable to set up the mmap'd file in that initializer. If that's not true, I'd appreciate a counterexample. For processes that don't use the runtime's static initializer, the situation (at Apple, at least) is typically that it's a kernel extension or bare-metal piece of firmware, and the continuous mode isn't really applicable anyway.

Oct 2 2019, 1:07 PM · Restricted Project, Restricted Project, Restricted Project
sajjadm added a comment to D68351: [profile] Add a mode to continuously sync counter updates to a file.

Not sure it belongs in this CL specifically, but would it be possible to later add a mode that defers the initialization of the mmap'd file until specifically requested by the process?

Oct 2 2019, 12:13 PM · Restricted Project, Restricted Project, Restricted Project

Jun 24 2019

sajjadm committed rG6694b2b36bdd: (Reland with changes) Adding a function for setting coverage output file. (authored by sajjadm).
(Reland with changes) Adding a function for setting coverage output file.
Jun 24 2019, 2:34 PM
sajjadm committed rL364231: (Reland with changes) Adding a function for setting coverage output file..
(Reland with changes) Adding a function for setting coverage output file.
Jun 24 2019, 2:33 PM
sajjadm closed D63581: (Reland with changes) Adding a function for setting coverage output file..
Jun 24 2019, 2:33 PM · Restricted Project, Restricted Project

Jun 19 2019

sajjadm updated the diff for D63581: (Reland with changes) Adding a function for setting coverage output file..

Ran clang-format.

Jun 19 2019, 5:10 PM · Restricted Project, Restricted Project
sajjadm updated the summary of D63581: (Reland with changes) Adding a function for setting coverage output file..
Jun 19 2019, 5:08 PM · Restricted Project, Restricted Project
sajjadm created D63581: (Reland with changes) Adding a function for setting coverage output file..
Jun 19 2019, 5:06 PM · Restricted Project, Restricted Project

Jun 6 2019

sajjadm added a comment to D62541: Adding a function for setting coverage output file..

LGTM (was OOO), thanks a lot David for Sajjad for multiple iterations here. One question: was the test landed? I see it in the latest patchset (https://reviews.llvm.org/D62541?id=203034), but not in the code that was committed.

Jun 6 2019, 2:04 PM · Restricted Project, Restricted Project

Jun 5 2019

sajjadm added a comment to D62541: Adding a function for setting coverage output file..

Thank you! I did apply for committer status today but I don't know how long it takes to get it.

Jun 5 2019, 4:39 PM · Restricted Project, Restricted Project
sajjadm added a comment to D62541: Adding a function for setting coverage output file..

Could you be more specific about your concerns? I'm happy to fix any problems in this change.

Jun 5 2019, 4:13 PM · Restricted Project, Restricted Project
sajjadm added a comment to D62541: Adding a function for setting coverage output file..

Could someone please commit for me?

Jun 5 2019, 2:18 PM · Restricted Project, Restricted Project

Jun 4 2019

sajjadm updated the diff for D62541: Adding a function for setting coverage output file..

Extracted lock/unlock code to util file, combined __llvm functions.

Jun 4 2019, 2:54 PM · Restricted Project, Restricted Project
sajjadm added inline comments to D62541: Adding a function for setting coverage output file..
Jun 4 2019, 2:22 PM · Restricted Project, Restricted Project
sajjadm updated the diff for D62541: Adding a function for setting coverage output file..

Ran clang-format.

Jun 4 2019, 11:26 AM · Restricted Project, Restricted Project
sajjadm updated the diff for D62541: Adding a function for setting coverage output file..

Forgot to declare a variable.

Jun 4 2019, 11:26 AM · Restricted Project, Restricted Project

Jun 3 2019

sajjadm updated the diff for D62541: Adding a function for setting coverage output file..

Fixed lock/unlock code.

Jun 3 2019, 6:27 PM · Restricted Project, Restricted Project
sajjadm updated the diff for D62541: Adding a function for setting coverage output file..

Added explicit merging API and test.

Jun 3 2019, 6:05 PM · Restricted Project, Restricted Project

May 30 2019

sajjadm added a comment to D62541: Adding a function for setting coverage output file..

On further thought, I don't think having a boolean that tells the runtime "user code is controlling locking so it is safe to merge" is a good idea, because typically __llvm_profile_write_file is invoked at exit. User code would have to ensure that it locks the file at the very end of its execution, when it has collected the desired profiling data, but before __lvm_profile_write_file is invoked. Also, in the current implementation, whether or not merging happens is controlled by the pattern of the profiling file name (whether or not it has a %m in it).

May 30 2019, 2:54 PM · Restricted Project, Restricted Project

May 29 2019

sajjadm added a comment to D62541: Adding a function for setting coverage output file..

@davidxl We could also have the openFileForMerging function invoke lprofLockFd on the user-provided file, rather than putting the onus for locking on the user. How would you feel about that?

May 29 2019, 4:26 PM · Restricted Project, Restricted Project
sajjadm updated the diff for D62541: Adding a function for setting coverage output file..

Updated test.

May 29 2019, 4:06 PM · Restricted Project, Restricted Project
sajjadm updated the diff for D62541: Adding a function for setting coverage output file..

Cleaned up logic and added safe-for-merge bool to API.

May 29 2019, 4:06 PM · Restricted Project, Restricted Project
sajjadm updated the diff for D62541: Adding a function for setting coverage output file..

Added support for merging, docs, and test case.

May 29 2019, 2:41 PM · Restricted Project, Restricted Project

May 28 2019

sajjadm added reviewers for D62541: Adding a function for setting coverage output file.: Dor1s, vsk, liaoyuke, davidxl.
May 28 2019, 11:48 AM · Restricted Project, Restricted Project
sajjadm created D62541: Adding a function for setting coverage output file..
May 28 2019, 11:44 AM · Restricted Project, Restricted Project

Mar 13 2019

sajjadm added a comment to D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..

I was debating whether I should pull -num-threads out of the docs for show and move it to the top-level options section, or make a copy of it in the export options section. I went with putting a copy of it in export because only show and export use it and the behavior is subtly different (show only uses it if -output-dir is set, export uses it unconditionally).

Mar 13 2019, 5:16 PM · Restricted Project
sajjadm updated the diff for D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..
  • Changed how -num-threads is documented.
Mar 13 2019, 5:13 PM · Restricted Project
sajjadm updated the diff for D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..
  • Added docs for new llvm-cov flags.
Mar 13 2019, 5:10 PM · Restricted Project
sajjadm added inline comments to D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..
Mar 13 2019, 5:10 PM · Restricted Project
sajjadm added a comment to D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..

Thanks for all the feedback, Max and Vedant!

Mar 13 2019, 4:20 PM · Restricted Project
sajjadm updated the diff for D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..
  • Added tests for the -skip-* flags.
  • Minor changes in response to code review.
Mar 13 2019, 4:13 PM · Restricted Project
sajjadm added a reviewer for D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation.: vsk.
Mar 13 2019, 10:21 AM · Restricted Project

Mar 12 2019

sajjadm updated the summary of D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..
Mar 12 2019, 4:07 PM · Restricted Project
sajjadm created D59277: Speeding up llvm-cov export with multithreaded renderFiles implementation..
Mar 12 2019, 4:06 PM · Restricted Project