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.

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

Improved std support for ps vita target #111819

Merged
merged 3 commits into from
Jun 7, 2023
Merged

Improved std support for ps vita target #111819

merged 3 commits into from
Jun 7, 2023

Conversation

nikarh
Copy link
Contributor

@nikarh nikarh commented May 21, 2023

Fixed a couple of things in std support for ps vita via Vita SDK newlib oss implementation:

  • Added missing hardware features to target spec
  • Compile in thumb by default (newlib is also compiled in thumb)
  • Fixed fs calls. Vita newlib has a not-very-posix dirent. Also vita does not expose inodes, it's stubbed as 0 in stat, and I'm stubbing it here for dirent (because vita newlibs's dirent doesn't even have that field)
  • Enabled signal handlers for panic unwinding
  • Dropped static link requirement from the platform support md. Also, rearranged sections to better stick with the template.

@rustbot
Copy link
Collaborator

rustbot commented May 21, 2023

r? @GuillaumeGomez

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 21, 2023
@rustbot
Copy link
Collaborator

rustbot commented May 21, 2023

These commits modify compiler targets.
(See the Target Tier Policy.)

@GuillaumeGomez
Copy link
Member

r? @Amanieu

@rustbot rustbot assigned Amanieu and unassigned GuillaumeGomez May 21, 2023
@nikarh
Copy link
Contributor Author

nikarh commented May 23, 2023

Found some issues related to incorrect bindings for vita in libc, converting this to a draft until I resolve it all.

@nikarh nikarh marked this pull request as draft May 23, 2023 14:36
bors added a commit to rust-lang/libc that referenced this pull request May 26, 2023
Fixed vita libc definitions

This fixes definitions that were incorrect in my initial PR (#3209). As with the previous one, this relies on open-source newlib implementation and doesn't use Sony internal API's or expose Sony internal data structures.

This fixes:
- fs API that uses `dirent` and stat
- sockets
- pthreads

A couple of explanations
- Unfortunately `dirent` in vita newlib is not very POSIX'y, before `d_name` it has a field with an internal data structure, which is of no use for std (no inodes, no file type inside), so I've just stubbed it as an `__offset`. Also, Vita doesn't expose inodes - `dirent` doesn't have it at all, and in `stat` it is always `0`. I am not sure what would be the best approach here. Maybe I should move the POSIX `dirent` to `generic.rs` and reexpose it in all targets, and redefine it in `vita/mod.rs`?
- All pthread types on Vita are pointers, and the backing data structure is allocated by corresponding init functions, so their sizeof's are all 4. I also changed `pthread_attr_t` and `pthread_rwlockattr_t` to reflect their sizes and not be constant. May be in relation to rust-lang/rust#95496 it would be better to move existing thread definitions to generic, and for vita specifically make them pointer types instead of byte arrays.

The fixes in std will be in rust-lang/rust#111819
@rust-log-analyzer

This comment has been minimized.

@nikarh
Copy link
Contributor Author

nikarh commented Jun 1, 2023

Waiting for libc to be released, then I'll bump the version and undraft the pr.

@rust-log-analyzer

This comment has been minimized.

@Amanieu
Copy link
Member

Amanieu commented Jun 1, 2023

Waiting for libc to be released, then I'll bump the version and undraft the pr.

You need to submit a PR to libc to bump the version number if you need a new release.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

bors added a commit to rust-lang/libc that referenced this pull request Jun 4, 2023
Update crate version to 0.2.145

#3255 is a prerequisite for rust-lang/rust#111819
@nikarh nikarh marked this pull request as ready for review June 4, 2023 15:07
@rustbot
Copy link
Collaborator

rustbot commented Jun 4, 2023

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@nikarh nikarh requested a review from Amanieu June 5, 2023 06:16
library/std/src/sys/unix/fs.rs Show resolved Hide resolved
library/std/src/sys/unix/net.rs Outdated Show resolved Hide resolved
@nikarh nikarh requested a review from Amanieu June 5, 2023 16:31
@Amanieu
Copy link
Member

Amanieu commented Jun 5, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jun 5, 2023

📌 Commit ac48d49 has been approved by Amanieu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 6, 2023
Improved std support for ps vita target

Fixed a couple of things in std support for ps vita via Vita SDK newlib oss implementation:

- Added missing hardware features to target spec
- Compile in thumb by default (newlib is also compiled in thumb)
- Fixed fs calls. Vita newlib has a not-very-posix dirent. Also vita does not expose inodes, it's stubbed as 0 in stat, and I'm stubbing it here for dirent (because vita newlibs's dirent doesn't even have that field)
- Enabled signal handlers for panic unwinding
- Dropped static link requirement from the platform support md. Also, rearranged sections to better stick with the template.
@Noratrieb
Copy link
Member

this probably caused a CI failure on musl on the rollup: #112336 (comment)
@bors r-
@bors rollup=iffy updating libc and changing platform dependent code

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 6, 2023
@nikarh
Copy link
Contributor Author

nikarh commented Jun 6, 2023

Seems to be related to libc upgrade and rust-lang/libc#2935

Unix definition of open64 used to have 2 args and varargs, now for musl it's 3 arguments.

@Amanieu
Copy link
Member

Amanieu commented Jun 6, 2023

Looks like rust-lang/libc#3264.

@Amanieu
Copy link
Member

Amanieu commented Jun 6, 2023

There is a PR in libc to fix this: rust-lang/libc#3265

Once it is merged, submit another PR to libc to update the version number (feel free to r? me).

@rustbot
Copy link
Collaborator

rustbot commented Jun 6, 2023

Failed to set assignee to me: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

@nikarh
Copy link
Contributor Author

nikarh commented Jun 6, 2023

Bumped libc version, an issue that caused CI failure on musl should be fixed.

@Amanieu
Copy link
Member

Amanieu commented Jun 6, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jun 6, 2023

📌 Commit 032857e has been approved by Amanieu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 6, 2023
@bors
Copy link
Contributor

bors commented Jun 7, 2023

⌛ Testing commit 032857e with merge b3dd578...

@bors
Copy link
Contributor

bors commented Jun 7, 2023

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing b3dd578 to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Jun 7, 2023

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing b3dd578 to master...

@bors bors added merged-by-bors This PR was explicitly merged by bors. labels Jun 7, 2023
@bors bors merged commit b3dd578 into rust-lang:master Jun 7, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 7, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b3dd578): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.8% [4.8%, 4.8%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 647.65s -> 647.662s (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants