Add QNX 7.0 x86 target by samkearney · Pull Request #1 · flba-eb/rust-lang-fork

samkearney · 2023-03-04T06:09:57Z

Add QNX 7.0 x86 target with no_std support only for now.

This is a preliminary PR for review before opening it against the main rust repo.

Successfully tested a no_std app on my x86 32-bit QNX 7.0 environment. It looks like running any part of the rust test suite requires std support for the target, so that will have to wait until I finish with the std support.

cc @gh-tr

flba-eb

@samkearney : Thank you very much for your contribution!

This looks good from my point of view. I cannot really review the target options like data_layout (same as in i686 linux gnu) and llvm_target (i586-pc-unknown).
@gh-tr: Is the same configuration used in qcc-clang? Any other thoughts?

In my opinion we should make this an official PR.

flba-eb · 2023-03-06T09:45:34Z

Not relevant yet in this PR, but we will later need to set environment variables:

diff --git a/src/doc/rustc/src/platform-support/nto-qnx.md b/src/doc/rustc/src/platform-support/nto-qnx.md
index 0d815c9b598..fd74ab5cbf7 100644
--- a/src/doc/rustc/src/platform-support/nto-qnx.md
+++ b/src/doc/rustc/src/platform-support/nto-qnx.md
@@ -117,7 +117,11 @@ export build_env='
     CC_x86_64-pc-nto-qnx710=qcc
     CFLAGS_x86_64-pc-nto-qnx710=-Vgcc_ntox86_64_cxx
     CXX_x86_64-pc-nto-qnx710=qcc
-    AR_x86_64_pc_nto_qnx710=ntox86_64-ar'
+    AR_x86_64_pc_nto_qnx710=ntox86_64-ar
+    CC_i586-pc-nto-qnx700=qcc
+    CFLAGS_i586-pc-nto-qnx700=-Vgcc_ntox86_cxx
+    CXX_i586-pc-nto-qnx700=qcc
+    AR_i586_pc_nto_qnx700=ntox86-ar'
 
 env $build_env \
     ./x.py build \
@@ -147,7 +151,11 @@ export build_env='
     CC_x86_64-pc-nto-qnx710=qcc
     CFLAGS_x86_64-pc-nto-qnx710=-Vgcc_ntox86_64_cxx
     CXX_x86_64-pc-nto-qnx710=qcc
-    AR_x86_64_pc_nto_qnx710=ntox86_64-ar'
+    AR_x86_64_pc_nto_qnx710=ntox86_64-ar
+    CC_i586-pc-nto-qnx700=qcc
+    CFLAGS_i586-pc-nto-qnx700=-Vgcc_ntox86_cxx
+    CXX_i586-pc-nto-qnx700=qcc
+    AR_i586_pc_nto_qnx700=ntox86-ar'
 
 # Disable tests that only work on the host or don't make sense for this target.
 # See also:

samkearney · 2023-03-13T20:04:39Z

@flba-eb Thanks for the feedback! 2 questions:

Should I wait for feedback from @gh-tr on your question about the target configuration?
When opening the PR against the rust repo, should I fill out the target policy criteria again like was done here: Add tier 3 no_std AArch64/x86_64 support for the QNX Neutrino RTOS rust-lang/rust#102701, or is it not necessary since QNX support in general has already been merged?

gh-tr

In general it looks fine. If I were adding this, I definitely would have added the other target architectures for completeness. I'm guessing your interesting doesn't extend outside of x86?

compiler/rustc_target/src/spec/i586_pc_nto_qnx700.rs

samkearney

If I were adding this, I definitely would have added the other target architectures for completeness. I'm guessing your interesting doesn't extend outside of x86?

Right now yes -- our support for other architectures like x86_64 will also include migrating to QNX 7.1, which you have already implemented support for. But the main reason is that I don't have any 7.0 images ready to test with on architectures besides x86, and I think the nice workflow with qemu and mkqnximage was added in SDP 7.1, so that doesn't help for 7.0. If you know of a quick way for me to spin up an emulated QNX 7.0 test environment for x86_64 and aarch64, I'd be happy to include them.

compiler/rustc_target/src/spec/i586_pc_nto_qnx700.rs

gh-tr · 2023-03-14T14:23:34Z

If I were adding this, I definitely would have added the other target architectures for completeness. I'm guessing your interesting doesn't extend outside of x86?

Right now yes -- our support for other architectures like x86_64 will also include migrating to QNX 7.1, which you have already implemented support for. But the main reason is that I don't have any 7.0 images ready to test with on architectures besides x86, and I think the nice workflow with qemu and mkqnximage was added in SDP 7.1, so that doesn't help for 7.0. If you know of a quick way for me to spin up an emulated QNX 7.0 test environment for x86_64 and aarch64, I'd be happy to include them.

Just took a peak and the source code for mkqnximage exists on the 700 branch and I use it internally on 7.0. I can take a look in swcenter for you and see if it's available. Do you use the email first.last@company.com as your myqnx login? If not, you can fire me an email with the login name you use. It lets me see what's available from your perspective.

I apologize for bringing this up as I realize it's a little extra work. If it doesn't end up being easy from your side, I can add the targets and update this diff.

flba-eb · 2023-03-14T15:44:11Z

@flba-eb Thanks for the feedback! 2 questions:

When opening the PR against the rust repo, should I fill out the target policy criteria again like was done here: Add tier 3 no_std AArch64/x86_64 support for the QNX Neutrino RTOS rust-lang/rust#102701, or is it not necessary since QNX support in general has already been merged?

No, this is not required. As you pointed out, QNX support is already there.

gh-tr · 2023-03-15T13:42:01Z

Just took a peak and the source code for mkqnximage exists on the 700 branch and I use it internally on 7.0. I can take a look in swcenter for you and see if it's available. Do you use the email first.last@company.com as your myqnx login? If not, you can fire me an email with the login name you use. It lets me see what's available from your perspective.

I apologize for bringing this up as I realize it's a little extra work. If it doesn't end up being easy from your side, I can add the targets and update this diff.

It appears that 7.0 mkqnximage is for internal use only. Just go ahead with x86. Other targets can be added another time.

samkearney · 2023-03-15T16:29:45Z

It appears that 7.0 mkqnximage is for internal use only. Just go ahead with x86. Other targets can be added another time.

OK! I will close this PR and reopen it against mainline Rust. Thanks for the review, guys

Stop turning transmutes into discriminant reads in mir-opt Partially reverts rust-lang#109612, as after rust-lang#109993 these aren't actually equivalent any more, and I'm no longer confident this was ever an improvement in the first place. Having this "simplification" meant that similar-looking code actually did somewhat different things. For example, ```rust pub unsafe fn demo1(x: std::cmp::Ordering) -> u8 { std::mem::transmute(x) } pub unsafe fn demo2(x: std::cmp::Ordering) -> i8 { std::mem::transmute(x) } ``` in nightly today is generating <https://rust.godbolt.org/z/dPK58zW18> ```llvm define noundef i8 `@_ZN7example5demo117h341ef313673d2ee6E(i8` noundef %x) unnamed_addr #0 { %0 = icmp uge i8 %x, -1 %1 = icmp ule i8 %x, 1 %2 = or i1 %0, %1 call void `@llvm.assume(i1` %2) ret i8 %x } define noundef i8 `@_ZN7example5demo217h5ad29f361a3f5700E(i8` noundef %0) unnamed_addr #0 { %x = alloca i8, align 1 store i8 %0, ptr %x, align 1 %1 = load i8, ptr %x, align 1, !range !2, !noundef !3 ret i8 %1 } ``` Which feels too different when the original code is essentially identical. --- Aside: that example is different *after* optimizations too: ```llvm define noundef i8 `@_ZN7example5demo117h341ef313673d2ee6E(i8` noundef returned %x) unnamed_addr #0 { %0 = add i8 %x, 1 %1 = icmp ult i8 %0, 3 tail call void `@llvm.assume(i1` %1) ret i8 %x } define noundef i8 `@_ZN7example5demo217h5ad29f361a3f5700E(i8` noundef returned %0) unnamed_addr #1 { ret i8 %0 } ``` so turning the `Transmute` into a `Discriminant` was arguably just making things worse, so leaving it alone instead -- and thus having less code in rustc -- seems clearly better.

Fixes rust-lang#111510 and complements rust-lang#106547 by adding support for encoding type parameters and also by transforming trait objects' traits into their identities before emitting type checks.

CFI: Fix encode_ty: unexpected Param(B/#1) Fixes rust-lang#111510 and complements rust-lang#106547 by adding support for encoding type parameters and also by transforming trait objects' traits into their identities before emitting type checks.

we get these declarations ``` ; opt level 0 declare x86_intrcc void @page_fault_handler(ptr byval([8 x i8]) align 8, i64) unnamed_addr #1 ; opt level > 0 declare x86_intrcc void @page_fault_handler(ptr noalias nocapture noundef byval([8 x i8]) align 8 dereferenceable(8), i64 noundef) unnamed_addr #1 ``` The space after `i64` in the original regex made the regex not match for opt level 0. Removing the space fixes the issue. ``` declare x86_intrcc void @page_fault_handler(ptr {{.*}}, i64 {{.*}}){{.*}}#[[ATTRS:[0-9]+]] ```

Add QNX 7.0 x86 target

33f7459

samkearney requested a review from flba-eb March 4, 2023 06:10

flba-eb approved these changes Mar 6, 2023

View reviewed changes

gh-tr reviewed Mar 13, 2023

View reviewed changes

compiler/rustc_target/src/spec/i586_pc_nto_qnx700.rs Show resolved Hide resolved

samkearney commented Mar 13, 2023

View reviewed changes

compiler/rustc_target/src/spec/i586_pc_nto_qnx700.rs Show resolved Hide resolved

samkearney closed this Mar 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add QNX 7.0 x86 target#1

Add QNX 7.0 x86 target#1
samkearney wants to merge 1 commit intomasterfrom
add-i586-qnx70-target

samkearney commented Mar 4, 2023 •

edited

Loading

Uh oh!

flba-eb left a comment

Uh oh!

flba-eb commented Mar 6, 2023

Uh oh!

samkearney commented Mar 13, 2023

Uh oh!

gh-tr left a comment

Uh oh!

Uh oh!

samkearney left a comment

Uh oh!

Uh oh!

gh-tr commented Mar 14, 2023

Uh oh!

flba-eb commented Mar 14, 2023

Uh oh!

gh-tr commented Mar 15, 2023

Uh oh!

samkearney commented Mar 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

samkearney commented Mar 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flba-eb left a comment

Choose a reason for hiding this comment

Uh oh!

flba-eb commented Mar 6, 2023

Uh oh!

samkearney commented Mar 13, 2023

Uh oh!

gh-tr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samkearney left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gh-tr commented Mar 14, 2023

Uh oh!

flba-eb commented Mar 14, 2023

Uh oh!

gh-tr commented Mar 15, 2023

Uh oh!

samkearney commented Mar 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

samkearney commented Mar 4, 2023 •

edited

Loading