Page MenuHomePhabricator

sivachandra (Siva Chandra)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 28 2014, 11:30 AM (267 w, 3 d)

Recent Activity

Tue, Dec 10

sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Tue, Dec 10, 9:13 AM · Restricted Project

Mon, Dec 9

sivachandra committed rG453c85ff0f96: [libc] Add implementation of errno and define the other macros of errno.h. (authored by sivachandra).
[libc] Add implementation of errno and define the other macros of errno.h.
Mon, Dec 9, 1:40 PM
sivachandra closed D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Mon, Dec 9, 1:40 PM · Restricted Project
sivachandra added a comment to D71094: [libc] Add implementation of errno and define the other macros of errno.h..

@theraven has been added as a reviewer. But, I prefer to commit this patch without waiting on his review. If @theraven has any comments, I will be happy to address them post review.

Mon, Dec 9, 1:31 PM · Restricted Project
sivachandra added a comment to D71094: [libc] Add implementation of errno and define the other macros of errno.h..

@abrachet It seems you have more comments that cannot be simply categorized as minor updates after you accepted the patch (within 40 minutes.after the patch was posted) Phabricator just simply marks the patch "green" unless another person marks it "Request Changes". There are some controversial parts that may give rise to public disagreement. For such cases, it might be better waiting a bit. (For this patch, I feel that if I did not click "Request Changes" in time, it would be committed and it would be more difficult to improve the situation later.)

I appreciate that you and @abrachet have volunteered to do the reviews here. At the same time, I find the tone of the above comment unfriendly. AFAICT, @abrachet is doing this voluntarily out of his own interest. An unfriendly tone for actions, which I view as "friendly community citizenship", definitely is uncalled for.

Apologize if you feel that my comments were not friendly. I did not intend that.

My reason to accept this now is that secondary issues like visibility are almost certainly going to need revisiting later, when we have more content to work with. For instance, do we do it by adding attributes everywhere in sight, or do we build some kind of script or other automation? Given that this is being developed incrementally from zero, we are just making extra unnecessary work by insisting that the defacto prototype solve all the problems that might need solving in the future. So let's get the core of the system figured out now, and not worry about secondary issues until more of the system is in place.

My feeling is that making the attribute change part of the patch is preferable. It makes the intention clear what symbols are public and what symbols and internal implementation details. Adding errno in an incremental step looks great to me. But making attribute changes separate is not. From the comments, it seems that __errno_location may be overlayed on top of an existing libc? I am a bit more concerned about the approach that will be used to do the overlay work. The design choice made here needs to account for standalone libc mode + overlay mode. I think we need to make the two modes clear.

Mon, Dec 9, 1:21 PM · Restricted Project
sivachandra added a comment to D71094: [libc] Add implementation of errno and define the other macros of errno.h..

@abrachet It seems you have more comments that cannot be simply categorized as minor updates after you accepted the patch (within 40 minutes.after the patch was posted) Phabricator just simply marks the patch "green" unless another person marks it "Request Changes". There are some controversial parts that may give rise to public disagreement. For such cases, it might be better waiting a bit. (For this patch, I feel that if I did not click "Request Changes" in time, it would be committed and it would be more difficult to improve the situation later.)

Sorry about that there ended up being many more changes than I anticipated here when I accepted. I'll be more careful next time.

Mon, Dec 9, 1:02 PM · Restricted Project
sivachandra added a comment to D71094: [libc] Add implementation of errno and define the other macros of errno.h..

@abrachet It seems you have more comments that cannot be simply categorized as minor updates after you accepted the patch (within 40 minutes.after the patch was posted) Phabricator just simply marks the patch "green" unless another person marks it "Request Changes". There are some controversial parts that may give rise to public disagreement. For such cases, it might be better waiting a bit. (For this patch, I feel that if I did not click "Request Changes" in time, it would be committed and it would be more difficult to improve the situation later.)

Mon, Dec 9, 11:39 AM · Restricted Project
sivachandra added a comment to D71094: [libc] Add implementation of errno and define the other macros of errno.h..

errno is a fundamental component baked in almost every component of a libc and contributors/reviewers should really be on the same page. Both abrachet and stanshebs accepted the patch and they were the initially-added reviewers. I feel a bit lonely to be the only one in the "Request Changes" camp, but I hope you do not commit the patch with these open issues not resolved. To reiterate, I feel strong about my suggested items:

  • __attribute__((visibility("hidden"))) in the internal header.
  • __attribute__((const)) in the public header.
Mon, Dec 9, 11:29 AM · Restricted Project
sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Mon, Dec 9, 10:34 AM · Restricted Project
sivachandra updated the diff for D71094: [libc] Add implementation of errno and define the other macros of errno.h..

Address abrachet's comments.

Mon, Dec 9, 10:15 AM · Restricted Project

Sun, Dec 8

sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Sun, Dec 8, 10:34 PM · Restricted Project

Fri, Dec 6

sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Fri, Dec 6, 3:39 PM · Restricted Project
sivachandra updated the diff for D71094: [libc] Add implementation of errno and define the other macros of errno.h..

Split the error macros in two POSIX and Linux based on https://pubs.opengroup.org/onlinepubs/9699919799/

Fri, Dec 6, 3:31 PM · Restricted Project
sivachandra added a comment to D71094: [libc] Add implementation of errno and define the other macros of errno.h..

For posterity, #define errno *__errno_location() breaks C11 7.1.2p5:

Any definition of an object-like macro described in this clause shall expand to code that is fully protected by parentheses where necessary, so that it groups in an arbitrary expression as if it were a single identifier.

It will fail to diagnose code like 3 errno[0] (false positive), and incorrectly flag (false negative):

const char *blah[] = { [EILSEQ] = "asdf" };
errno = EILSEQ;
puts(errno[blah]);

and errno++

Fri, Dec 6, 11:47 AM · Restricted Project
sivachandra updated the diff for D71094: [libc] Add implementation of errno and define the other macros of errno.h..

Instead of defining the error macros, use the ones available in linux/errno.h

Fri, Dec 6, 11:47 AM · Restricted Project
sivachandra updated the diff for D71094: [libc] Add implementation of errno and define the other macros of errno.h..

Address @MaskRay and @alexbrachet's comments.

Fri, Dec 6, 1:27 AM · Restricted Project
sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Fri, Dec 6, 1:13 AM · Restricted Project
sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Fri, Dec 6, 12:59 AM · Restricted Project
sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Fri, Dec 6, 12:45 AM · Restricted Project
sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Fri, Dec 6, 12:36 AM · Restricted Project

Thu, Dec 5

sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Thu, Dec 5, 11:47 PM · Restricted Project
sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Thu, Dec 5, 11:38 PM · Restricted Project
sivachandra retitled D71094: [libc] Add implementation of errno and define the other macros of errno.h. from [libc] Add implementation of errno. to [libc] Add implementation of errno and define the other macros of errno.h..
Thu, Dec 5, 11:29 PM · Restricted Project
sivachandra added inline comments to D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Thu, Dec 5, 11:29 PM · Restricted Project
sivachandra updated the diff for D71094: [libc] Add implementation of errno and define the other macros of errno.h..

Address comments.

Thu, Dec 5, 11:22 PM · Restricted Project
sivachandra created D71094: [libc] Add implementation of errno and define the other macros of errno.h..
Thu, Dec 5, 3:03 PM · Restricted Project

Fri, Nov 22

sivachandra abandoned D69421: [libc] Header generation scheme..

Abandoning this change now as the patch implementing the ideas showcased here has landed.

Fri, Nov 22, 1:38 PM · Restricted Project
sivachandra added a comment to D70197: [libc] Add a TableGen based header generator..

I think this is in a good spot, it's easy to read and modify later as needs change. I don't know much about TableGen, I've tried my best to look through, but it would be preferable if other reviewers would be able to take a look too. Up to your discretion how long you would like to wait to let others chime in. LGTM

Fri, Nov 22, 1:11 PM · Restricted Project
sivachandra committed rGb47f9eb55d18: [libc] Add a TableGen based header generator. (authored by sivachandra).
[libc] Add a TableGen based header generator.
Fri, Nov 22, 1:03 PM
sivachandra closed D70197: [libc] Add a TableGen based header generator..
Fri, Nov 22, 1:03 PM · Restricted Project
sivachandra added inline comments to D70197: [libc] Add a TableGen based header generator..
Fri, Nov 22, 10:09 AM · Restricted Project
sivachandra updated the diff for D70197: [libc] Add a TableGen based header generator..

Address latest round of comments.

Fri, Nov 22, 10:09 AM · Restricted Project

Wed, Nov 20

sivachandra added inline comments to D70197: [libc] Add a TableGen based header generator..
Wed, Nov 20, 12:47 PM · Restricted Project
sivachandra updated the diff for D70197: [libc] Add a TableGen based header generator..

Fix few copy-paste errors pointed out by abrachet; Get rid of Command::Status.

Wed, Nov 20, 12:42 PM · Restricted Project

Tue, Nov 19

sivachandra added a comment to D70197: [libc] Add a TableGen based header generator..

Not of huge importance, more of curiosity, but could you do what you did with a past revision where you pushed a review with an svg file to your fork so we could see what it looks like on your GitHub?

Tue, Nov 19, 11:46 PM · Restricted Project
sivachandra added inline comments to D70197: [libc] Add a TableGen based header generator..
Tue, Nov 19, 11:35 PM · Restricted Project
sivachandra updated the diff for D70197: [libc] Add a TableGen based header generator..

Address comments.

Tue, Nov 19, 11:35 PM · Restricted Project
sivachandra updated the diff for D70197: [libc] Add a TableGen based header generator..

Improve some comments.

Tue, Nov 19, 12:24 PM · Restricted Project
sivachandra accepted D70416: [Driver] Make -static-libgcc imply static libunwind.

Please wait for saugustine's acceptance.

Tue, Nov 19, 9:20 AM · Restricted Project

Thu, Nov 14

sivachandra added a reviewer for D70197: [libc] Add a TableGen based header generator.: mcgrathr.
Thu, Nov 14, 4:09 PM · Restricted Project

Nov 13 2019

sivachandra updated the diff for D70197: [libc] Add a TableGen based header generator..

Use the new header generator to generate string.h.

Nov 13 2019, 1:10 PM · Restricted Project
sivachandra created D70197: [libc] Add a TableGen based header generator..
Nov 13 2019, 11:38 AM · Restricted Project

Nov 4 2019

sivachandra added a comment to D69727: [libc builder] Use AnnotatedBuilder instead of LLVMBuilder..

Hello Siva,

How AnnotatedBuilder handles clean arg doesn't seem related to adding libc builders. May I suggest removing this change from your patch and make a separate review item if you want to change that behavior, please?

Nov 4 2019, 10:13 PM
sivachandra updated the diff for D69727: [libc builder] Use AnnotatedBuilder instead of LLVMBuilder..

Use the module libc_vars.

Nov 4 2019, 10:13 PM

Nov 3 2019

sivachandra added inline comments to D69421: [libc] Header generation scheme..
Nov 3 2019, 11:11 PM · Restricted Project

Nov 1 2019

sivachandra committed rZORG12f995533f41: Remove the libc builders from the master for now. (authored by sivachandra).
Remove the libc builders from the master for now.
Nov 1 2019, 12:23 PM
sivachandra closed D69728: Remove the libc builders from the master for now..
Nov 1 2019, 12:23 PM
sivachandra created D69728: Remove the libc builders from the master for now..
Nov 1 2019, 12:16 PM
sivachandra added a comment to D69727: [libc builder] Use AnnotatedBuilder instead of LLVMBuilder..

This part of the fix you wanted me to do in https://reviews.llvm.org/D69655

Nov 1 2019, 12:04 PM
sivachandra created D69727: [libc builder] Use AnnotatedBuilder instead of LLVMBuilder..
Nov 1 2019, 12:04 PM
sivachandra added a comment to D67933: Update llvm-libc status with code repo and mailing list information..

Will put this in on Monday if there no objections.

Nov 1 2019, 11:18 AM · Restricted Project
sivachandra updated the diff for D67933: Update llvm-libc status with code repo and mailing list information..

Do not talk about the SVN repo.

Nov 1 2019, 11:16 AM · Restricted Project
sivachandra committed rG9364107cf348: Illustrate a redirector using the example of round function from math.h. (authored by sivachandra).
Illustrate a redirector using the example of round function from math.h.
Nov 1 2019, 11:08 AM
sivachandra closed D69020: Illustrate a redirector using the example of round function from math.h..
Nov 1 2019, 11:07 AM · Restricted Project
sivachandra committed rZORG6d5695aea629: Add llvm-libc builders. (authored by sivachandra).
Add llvm-libc builders.
Nov 1 2019, 9:59 AM
sivachandra closed D69655: Add llvm-libc builders..
Nov 1 2019, 9:59 AM

Oct 31 2019

sivachandra added inline comments to D69020: Illustrate a redirector using the example of round function from math.h..
Oct 31 2019, 11:16 AM · Restricted Project
sivachandra updated the diff for D69020: Illustrate a redirector using the example of round function from math.h..

Address comments

Oct 31 2019, 11:09 AM · Restricted Project

Oct 30 2019

sivachandra added a reviewer for D69655: Add llvm-libc builders.: vitalybuka.
Oct 30 2019, 10:56 PM
sivachandra created D69655: Add llvm-libc builders..
Oct 30 2019, 10:50 PM
sivachandra added a comment to D69421: [libc] Header generation scheme..

My latest diff showcases function attributes and also argument annotations.

Oct 30 2019, 12:19 PM · Restricted Project
sivachandra updated the diff for D69421: [libc] Header generation scheme..

Add example of function attrbutes and argument annotations.

Oct 30 2019, 12:10 PM · Restricted Project
sivachandra added inline comments to D69421: [libc] Header generation scheme..
Oct 30 2019, 8:53 AM · Restricted Project

Oct 29 2019

sivachandra added inline comments to D69421: [libc] Header generation scheme..
Oct 29 2019, 11:35 PM · Restricted Project
sivachandra updated the diff for D69421: [libc] Header generation scheme..

Use TableGen types instead of strings.

Oct 29 2019, 11:35 PM · Restricted Project
sivachandra added inline comments to D69421: [libc] Header generation scheme..
Oct 29 2019, 3:41 PM · Restricted Project
sivachandra updated the diff for D69421: [libc] Header generation scheme..

Add more examples.

Oct 29 2019, 3:40 PM · Restricted Project

Oct 28 2019

sivachandra updated the summary of D69020: Illustrate a redirector using the example of round function from math.h..
Oct 28 2019, 3:01 PM · Restricted Project
sivachandra added a comment to D69020: Illustrate a redirector using the example of round function from math.h..

I guess my question is more, whats the evidence that there isn't a way to avoid the two call overhead? The use case makes sense to me but my impression is that __round_redirector could be declared to have extern "C" linkage and then llvm_libc::round can be an alias for it. That gets rid of 1 layer of indirection. Getting rid of the next call seems more gross to me but I think some additional objcopy magic can be done to make __round_redirector an alias for ::round then I think this would all link correctly, right?

Oct 28 2019, 2:49 PM · Restricted Project

Oct 25 2019

sivachandra added inline comments to D69421: [libc] Header generation scheme..
Oct 25 2019, 3:34 PM · Restricted Project
sivachandra added inline comments to D69421: [libc] Header generation scheme..
Oct 25 2019, 3:06 PM · Restricted Project
sivachandra added a comment to D69020: Illustrate a redirector using the example of round function from math.h..

I've missed some emails and the round table at llvm. Could you describe more about why this mechanism for redirecting is used? It seems to me that there should be a way for LLVM_LIBC_ENTRYPOINT(round) to be an alias for ::round rather than requiring two (likely tail) calls here. Could you motivate this mechanism?

Oct 25 2019, 2:56 PM · Restricted Project

Oct 24 2019

sivachandra created D69421: [libc] Header generation scheme..
Oct 24 2019, 10:09 PM · Restricted Project
sivachandra added inline comments to D69341: [zorg] Port LLDB cmake build factory to git.
Oct 24 2019, 8:48 AM

Oct 23 2019

sivachandra added inline comments to D69341: [zorg] Port LLDB cmake build factory to git.
Oct 23 2019, 3:12 PM
sivachandra added inline comments to D69341: [zorg] Port LLDB cmake build factory to git.
Oct 23 2019, 1:50 PM
sivachandra added inline comments to D69341: [zorg] Port LLDB cmake build factory to git.
Oct 23 2019, 1:41 PM
sivachandra added inline comments to D69341: [zorg] Port LLDB cmake build factory to git.
Oct 23 2019, 12:23 PM

Oct 15 2019

sivachandra added a comment to D69020: Illustrate a redirector using the example of round function from math.h..

This patch illustrates one the desired "features" as proposed in http://llvm.org/docs/Proposals/LLVMLibC.html

Oct 15 2019, 11:23 PM · Restricted Project
sivachandra created D69020: Illustrate a redirector using the example of round function from math.h..
Oct 15 2019, 11:12 PM · Restricted Project
sivachandra committed rGc1157d1e77c3: [libc] Do not add unittests if LLVM_INCLUDE_TESTS is OFF. (authored by sivachandra).
[libc] Do not add unittests if LLVM_INCLUDE_TESTS is OFF.
Oct 15 2019, 10:49 AM
sivachandra closed D68726: [libc] Do not add unittests LLVM_INCLUDE_TESTS is OFF..
Oct 15 2019, 10:49 AM · Restricted Project
sivachandra committed rL374925: [libc] Do not add unittests if LLVM_INCLUDE_TESTS is OFF..
[libc] Do not add unittests if LLVM_INCLUDE_TESTS is OFF.
Oct 15 2019, 10:40 AM

Oct 11 2019

sivachandra created D68904: Add a high level target for libc unit tests called 'libc_unittests'..
Oct 11 2019, 10:48 PM · Restricted Project

Oct 10 2019

sivachandra committed rG7a6d98325cd7: Use arrays on stack and avoid use of new and delete operators. (authored by sivachandra).
Use arrays on stack and avoid use of new and delete operators.
Oct 10 2019, 9:09 AM
sivachandra committed rL374374: Use arrays on stack and avoid use of new and delete operators..
Use arrays on stack and avoid use of new and delete operators.
Oct 10 2019, 9:08 AM
sivachandra closed D68761: Use arrays on stack and avoid use of new and delete operators..
Oct 10 2019, 9:08 AM · Restricted Project

Oct 9 2019

sivachandra created D68761: Use arrays on stack and avoid use of new and delete operators..
Oct 9 2019, 10:47 PM · Restricted Project
sivachandra added a comment to D68726: [libc] Do not add unittests LLVM_INCLUDE_TESTS is OFF..

Hopefully this should fix the problem you reported here: https://reviews.llvm.org/D67867

Oct 9 2019, 1:41 PM · Restricted Project
sivachandra created D68726: [libc] Do not add unittests LLVM_INCLUDE_TESTS is OFF..
Oct 9 2019, 1:37 PM · Restricted Project

Oct 8 2019

sivachandra added a comment to D67867: [libc] Add few docs and implementation of strcpy and strcat..

That this is now committed does not change anything w.r.t. needing to respond to outstanding comments.

Oct 8 2019, 11:25 AM · Restricted Project, Restricted Project

Oct 4 2019

sivachandra committed rG4380647e79bd: Add few docs and implementation of strcpy and strcat. (authored by sivachandra).
Add few docs and implementation of strcpy and strcat.
Oct 4 2019, 10:33 AM
sivachandra committed rL373764: Add few docs and implementation of strcpy and strcat..
Add few docs and implementation of strcpy and strcat.
Oct 4 2019, 10:29 AM
sivachandra closed D67867: [libc] Add few docs and implementation of strcpy and strcat..
Oct 4 2019, 10:29 AM · Restricted Project, Restricted Project

Oct 3 2019

Herald added a project to D67867: [libc] Add few docs and implementation of strcpy and strcat.: Restricted Project.

My earlier question is about why we need the namespace __llvm_libc at all. From libc/src/string/strcat/strcat_test.cpp I conclude it is for unit testing in an environment that already has a libc (gtest). This should probably be documented.

Oct 3 2019, 10:10 PM · Restricted Project, Restricted Project

Oct 2 2019

sivachandra updated the diff for D67867: [libc] Add few docs and implementation of strcpy and strcat..

Move markdown docs to reStructuredText; Will add conf.py and build targets in a later pass.

Oct 2 2019, 11:06 PM · Restricted Project, Restricted Project
sivachandra added a comment to D67867: [libc] Add few docs and implementation of strcpy and strcat..

The issue really only exists when you refer to object files across standards layers -- e.g., using an object file that exposes the POSIX symbol "open" from an object file implementing ISO C. If you make sure to always strictly-layer the libc, so that ISO C-implementing object files don't use any POSIX-exporting object files, and so on, you won't need to mark anything weak. For the example of "open", you'd have an internal implementation of open in its own file, only exposing libc-internal symbols. Then fopen (ISO C) can use that safely, without dragging in a definition of the symbol open. Separately, the implementation of open (POSIX) can be defined in its own file, also based on the internal open.)

Oct 2 2019, 2:33 PM · Restricted Project, Restricted Project
sivachandra added a comment to D67867: [libc] Add few docs and implementation of strcpy and strcat..

Aha. But still, all but few docs migrated back to rst.
I'm not sure it's great to add new ones in the format being-migrated-from.

Oct 2 2019, 10:33 AM · Restricted Project, Restricted Project
sivachandra added a comment to D67867: [libc] Add few docs and implementation of strcpy and strcat..

The objcopy step is required to avoid putting mangled names with the alias attribute. If there is any other way to achieve the same thing, I am open to it.

Ah, I see! I'd suggest using extern "C" instead. There's no need for these be C++-mangled -- you can simply use a name prefix instead. E.g., if you define it as extern "C" __llvm_libc_strcpy(...) {} then it's trivial to make the strcpy alias without objcopy magic.

Oct 2 2019, 10:09 AM · Restricted Project, Restricted Project

Sep 30 2019

sivachandra added a comment to D67867: [libc] Add few docs and implementation of strcpy and strcat..

Actually, now that I think I understand the existing proposal better, I believe it's broken, as well as confusing. It's getting the same effect as using __attribute__((alias)), except harder to understand. But it's not ok to have a single object file expose both a strong public alias and an internal alias, for any function that's not in the baseline ISO C standard. It would be OK if the aliases were weak, or if they were strong but exposed by a separate .o file. (In any case, I'd like to suggest not using an external objcopy invocation to achieve this.)

Sep 30 2019, 12:06 PM · Restricted Project, Restricted Project