Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[libc][gnu ext] implement pthread_rwlockattr_{get|set}kind_np #89443

Closed
nickdesaulniers opened this issue Apr 19, 2024 · 9 comments · Fixed by #90249
Closed

[libc][gnu ext] implement pthread_rwlockattr_{get|set}kind_np #89443

nickdesaulniers opened this issue Apr 19, 2024 · 9 comments · Fixed by #90249
Assignees
@HendrikHuebner
Labels
libc

Comments

@nickdesaulniers
Copy link
Member

nickdesaulniers commented Apr 19, 2024 •
edited
Loading

These functions are perhaps useful. Not implemented in #89322 because I didn't know they existed outside of POSIX.

Since these aren't POSIX but instead gnu extensions, these should go in FOO rather than libc/spec/posix.td; we already have an entry for pthreads.h in that file. It will be easier for someone to start on this once #89322 lands.

I would extend our definition of pthread_rwlockattr_t in libc/include/llvm-libc-types/pthread_rwlockattr_t.h to include a new int member named pref such that these functions are simple getters and setters.

Then it sounds like we should define 3 macros (in libc/include/pthread.h.def, though keep an eye on #88997, too)

  • PTHREAD_RWLOCK_PREFER_READER_NP
  • PTHREAD_RWLOCK_PREFER_WRITER_NP
  • PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
@nickdesaulniers nickdesaulniers added good first issue https://github.com/llvm/llvm-project/contribute libc labels Apr 19, 2024
@llvmbot
Copy link

llvmbot commented Apr 19, 2024

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link

llvmbot commented Apr 19, 2024

@llvm/issue-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

These functions are perhaps useful. Not implemented in #89322 because I didn't know they existed outside of POSIX.

Since these aren't POSIX but instead gnu extensions, these should go in FOO rather than libc/spec/posix.td; we already have an entry for pthreads.h in that file. It will be easier for someone to start on this once #89322 lands.

I would extend our definition of pthread_rwlockattr_t in libc/include/llvm-libc-types/pthread_rwlockattr_t.h to include a new int member named pref such that these functions are simple getters and setters.

Then it sounds like we should define 3 macros (in libc/include/pthread.h.def, though keep an eye on #88997, too)

  • PTHREAD_RWLOCK_PREFER_READER_NP
  • PTHREAD_RWLOCK_PREFER_WRITER_NP
  • PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP

@llvmbot
Copy link

llvmbot commented Apr 19, 2024

@llvm/issue-subscribers-good-first-issue

Author: Nick Desaulniers (nickdesaulniers)

These functions are perhaps useful. Not implemented in #89322 because I didn't know they existed outside of POSIX.

Since these aren't POSIX but instead gnu extensions, these should go in FOO rather than libc/spec/posix.td; we already have an entry for pthreads.h in that file. It will be easier for someone to start on this once #89322 lands.

I would extend our definition of pthread_rwlockattr_t in libc/include/llvm-libc-types/pthread_rwlockattr_t.h to include a new int member named pref such that these functions are simple getters and setters.

Then it sounds like we should define 3 macros (in libc/include/pthread.h.def, though keep an eye on #88997, too)

  • PTHREAD_RWLOCK_PREFER_READER_NP
  • PTHREAD_RWLOCK_PREFER_WRITER_NP
  • PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP

@HendrikHuebner
Copy link
Contributor

I would like to work on it if possible.

@nickdesaulniers
Copy link
Member Author

@HendrikHuebner cool! Let me know if you get stuck. Note: I'll be offline a bit soon ish, so expected delayed review+feedback from me.

@HendrikHuebner HendrikHuebner mentioned this issue Apr 26, 2024
Merged
SchrodingerZhu pushed a commit that referenced this issue May 13, 2024
@HendrikHuebner
[libc][POSIX][pthreads] implemented missing pthread_rwlockattr functi…
d2676a7 
…ons (#90249)

Closes #89443

I added the two missing functions and respective test cases. Let me know
if anything needs changing.
@nickdesaulniers
Copy link
Member Author

Hi @SchrodingerZhu thanks for the quick revert.

Did this issue ever get fixed up? If not looks like we should re-open it?

@nickdesaulniers nickdesaulniers removed the good first issue https://github.com/llvm/llvm-project/contribute label May 20, 2024
@SchrodingerZhu
Copy link
Contributor

Hi @SchrodingerZhu thanks for the quick revert.

Did this issue ever get fixed up? If not looks like we should re-open it?

I agree. I don't think it is fixed by now.

@HendrikHuebner
Copy link
Contributor

HendrikHuebner commented May 21, 2024 •
edited
Loading

Sorry I haven't gotten around to it yet. I'll update it tomorrow. Somehow one of the files I changed wasn't committed.

This was referenced May 23, 2024
Closed
Merged
nickdesaulniers pushed a commit that referenced this issue May 30, 2024
@HendrikHuebner
Reland: [libc][POSIX][pthreads] implemented missing pthread_rwlockatt…
485f9f5 
…r functions (#93622)

New pull request for #89443

The previous PR was reverted after breaking fullbuild due to a missing
struct declaration, which I forgot to commit.

Reverts revert and adds the missing pthread_rwlockattr_getkind_np /
pthread_rwlockattr_setkind_np functions and tests respecitvely.
@nickdesaulniers
Copy link
Member Author

fixed: 485f9f5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@HendrikHuebner HendrikHuebner

Labels
libc
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[libc][POSIX][pthreads] implemented missing pthread_rwlockattr functions HendrikHuebner/llvm-project
4 participants
@nickdesaulniers @SchrodingerZhu @llvmbot @HendrikHuebner