Skip to content

Conversation

@nyurik
Copy link
Contributor

@nyurik nyurik commented Jul 18, 2024

This backports the QNX 7.1 aarch64 implementation to 7.0.

This PR bumps libc dependency to 0.2.158

CC: to the folks who did the initial implementation: @flba-eb, @gh-tr, @jonathanpallant, @japaric

Compile target

Note that until qnx700 gets properly released, use BOOTSTRAP_SKIP_TARGET_SANITY=1

# Configure qcc build environment
source _path_/_to_/qnx7.0/qnxsdp-env.sh

# Tell rust to use qcc when building QNX 7.0 targets
export build_env='
    BOOTSTRAP_SKIP_TARGET_SANITY=1
    CC_aarch64-unknown-nto-qnx700=qcc
    CFLAGS_aarch64-unknown-nto-qnx700=-Vgcc_ntoaarch64le_cxx
    CXX_aarch64-unknown-nto-qnx700=qcc
    AR_aarch64_unknown_nto_qnx700=ntoaarch64-ar'

# Build rust compiler, libs, and the remote test server
env $build_env ./x.py build \
  --target x86_64-unknown-linux-gnu,aarch64-unknown-nto-qnx700 \
  rustc library/core library/alloc library/std src/tools/remote-test-server

rustup toolchain link stage1 build/host/stage1

Compile "hello world"

source _path_/_to_/qnx7.0/qnxsdp-env.sh

cargo new hello_world
cd hello_world
cargo +stage1 build --release --target aarch64-unknown-nto-qnx700

Configure a remote for testing

Do this from a new shell - we will need to run more commands in the previous one. I ran into these two issues, and found some workarounds.

  • Temporary dir might not work properly
  • Default remote-test-server has issues binding to an address
# ./remote-test-server
starting test server
thread 'main' panicked at src/tools/remote-test-server/src/main.rs:175:29:
called `Result::unwrap()` on an `Err` value: Os { code: 249, kind: AddrNotAvailable, message: "Can't assign requested address" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Specifying --bind param actually fixes that, and so does setting TMPDIR properly.

# Copy remote-test-server to remote device. You may need to use sftp instead.
# ATTENTION: Note that the path is different from the one in the remote testing documentation for some reason
scp ./build/x86_64-unknown-linux-gnu/stage1-tools-bin/remote-test-server  qnxdevice:/path/

# Connect to the remote device to start the remote tester
# The port forwarding (-L) is only needed if the device is not accessible directly via TCP/IP.
# Port forwarding allows the tester to connect to the local port instead
ssh -L 12345:127.0.0.1:12345 qnxdevice

# on the device, run
rm -rf tmp && mkdir -p tmp && TMPDIR=$PWD/tmp ./remote-test-server --bind 0.0.0.0:12345

Run test suit

Assume all previous environment variables are still set, or re-init them

export TEST_DEVICE_ADDR="localhost:12345"

# tidy needs to be skipped due to using un-published libc dependency
export exclude_tests='
    --exclude src/bootstrap
    --exclude src/tools/error_index_generator
    --exclude src/tools/linkchecker
    --exclude src/tools/tidy
    --exclude tests/ui-fulldeps
    --exclude rustc
    --exclude rustdoc
    --exclude tests/run-make-fulldeps'

env $build_env ./x.py test  $exclude_tests --stage 1 --target aarch64-unknown-nto-qnx700

try-job: dist-x86_64-msvc

@rustbot
Copy link
Collaborator

rustbot commented Jul 18, 2024

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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 Jul 18, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@nyurik nyurik changed the title WIP: add aarch64_unknown_nto_qnx700 target - QNX 7.0 support for aarch64le add aarch64_unknown_nto_qnx700 target - QNX 7.0 support for aarch64le Jul 18, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@TaKO8Ki
Copy link
Member

TaKO8Ki commented Jul 18, 2024

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2024
@nyurik nyurik marked this pull request as ready for review July 19, 2024 05:06
@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2024

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

@nyurik
Copy link
Contributor Author

nyurik commented Jul 19, 2024

@TaKO8Ki I think this PR is ready - as it can be merged without waiting for libc. Thx!

@nyurik
Copy link
Contributor Author

nyurik commented Jul 19, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 19, 2024
@nyurik nyurik force-pushed the add-qnx-70-target branch from 64c6cd4 to 367f5ea Compare July 20, 2024 15:37
@rustbot
Copy link
Collaborator

rustbot commented Jul 20, 2024

Some changes occurred in src/doc/rustc/src/platform-support

cc @Nilstrieb

@rust-log-analyzer

This comment has been minimized.

@nyurik nyurik force-pushed the add-qnx-70-target branch from 367f5ea to 82e4590 Compare July 20, 2024 16:05
@nyurik nyurik force-pushed the add-qnx-70-target branch from 82e4590 to 53d070a Compare July 20, 2024 16:26
@bors
Copy link
Collaborator

bors commented Aug 30, 2024

📌 Commit f41e0bb has been approved by saethlin

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 Aug 30, 2024
@bors
Copy link
Collaborator

bors commented Sep 1, 2024

⌛ Testing commit f41e0bb with merge 1a1cc05...

@bors
Copy link
Collaborator

bors commented Sep 1, 2024

☀️ Test successful - checks-actions
Approved by: saethlin
Pushing 1a1cc05 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 1, 2024
@bors bors merged commit 1a1cc05 into rust-lang:master Sep 1, 2024
@rustbot rustbot added this to the 1.83.0 milestone Sep 1, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1a1cc05): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.4% [0.4%, 0.4%] 1
Regressions ❌
(secondary)
0.7% [0.5%, 1.2%] 4
Improvements ✅
(primary)
-1.8% [-3.3%, -0.3%] 2
Improvements ✅
(secondary)
-0.4% [-0.4%, -0.4%] 1
All ❌✅ (primary) -1.1% [-3.3%, 0.4%] 3

Max RSS (memory usage)

Results (primary 0.5%, secondary 0.4%)

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.9% [0.4%, 2.9%] 45
Regressions ❌
(secondary)
0.8% [0.4%, 2.4%] 58
Improvements ✅
(primary)
-0.7% [-1.2%, -0.4%] 16
Improvements ✅
(secondary)
-0.8% [-1.6%, -0.4%] 20
All ❌✅ (primary) 0.5% [-1.2%, 2.9%] 61

Cycles

Results (primary 0.1%, secondary -0.1%)

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.8% [0.4%, 3.7%] 30
Regressions ❌
(secondary)
0.8% [0.4%, 3.5%] 55
Improvements ✅
(primary)
-1.1% [-2.9%, -0.5%] 17
Improvements ✅
(secondary)
-1.1% [-5.4%, -0.4%] 52
All ❌✅ (primary) 0.1% [-2.9%, 3.7%] 47

Binary size

Results (primary -0.7%)

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)
- - 0
Improvements ✅
(primary)
-0.7% [-0.7%, -0.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.7% [-0.7%, -0.7%] 1

Bootstrap: 789.43s -> 790.965s (0.19%)
Artifact size: 338.46 MiB -> 338.44 MiB (-0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Sep 1, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Sep 1, 2024

@flba-eb
Copy link
Contributor

flba-eb commented Sep 1, 2024

Perf collector seems messed up atm, cf. https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance/topic/Weird.20perf.20results

I agree. My guess is the "performance regression" is only measurement noise as adding an additional target should not have any (measurable) performance impact. The only change which is not limited to the QNX Neutrino (nto) OS is upgrading the libc crate.

@jieyouxu
Copy link
Member

jieyouxu commented Sep 1, 2024

AFAICT currently the perf bot results really isn't indicating much of anything because perf history got messed up since #129714 (regression, improvement or unchanged), even if your changes are perf-significant. Nothing to do here without another perf run after fixing the perf bot.

@jieyouxu
Copy link
Member

jieyouxu commented Sep 3, 2024

Since the perf collector seems to have been repaired,

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 3, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Sep 3, 2024

@bors ping

@saethlin
Copy link
Member

saethlin commented Sep 3, 2024

Each commit hash can only be benchmarked once, so I don't think what you're looking for here is going to happen.

@lqd
Copy link
Member

lqd commented Sep 3, 2024

The significance thresholds have been updated on the collector. That is, even if the posted GH summary didn't change, the linked results have been updated.

image

And I'm pretty sure deep-vector was noisy at the time (and was "fixed" in the next merge). As mentioned earlier, this PR was unlikely to have introduced a regression anyways, other than incidentally via the libc bump, and that didn't happen.

I don't think anyone needs to spend more time on this, @jieyouxu, if you wanted to have updated results. We can mark this as triaged IMO, @rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Sep 3, 2024
@nyurik
Copy link
Contributor Author

nyurik commented Oct 27, 2024

It seems changes to supported platforms are part of relnotes, so adding the right label.

@rustbot label: +relnotes

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. O-neutrino OS: QNX Neutrino, a POSIX-compatible real-time operating system O-unix Operating system: Unix-like perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-perf Status: Waiting on a perf run to be completed. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.