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

Add support for C/C++ compiler for Neutrino QNX: qcc #1319

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

flba-eb
Copy link
Contributor

@flba-eb flba-eb commented Dec 12, 2024

This adds support for using the C/C++ compiler qcc from Neutrino QNX, with the target parameters required.

So far, it was required to set various environment variables.

CC: @jonathanpallant @japaric @gh-tr @AkhilTThomas

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, is it possible to setup CI and a new test to actually use qcc for testing

@flba-eb
Copy link
Contributor Author

flba-eb commented Dec 12, 2024

Unfortunately this is not possible because qcc requires a paid license.

@jonathanpallant
Copy link

jonathanpallant commented Dec 12, 2024

Is this going to let us drop all the CFLAGS_<target> environment variables from the QNX Neutrino libstd builds?

Ah, the link says yes.

@flba-eb
Copy link
Contributor Author

flba-eb commented Dec 12, 2024

Is this going to let us drop all the CFLAGS_<target> environment variables from the QNX Neutrino libstd builds?

Yes, see also rust-lang/rust#134211 😄

src/lib.rs Outdated Show resolved Hide resolved
@NobodyXu
Copy link
Collaborator

Not very familiar with Neutrino QNX, cc @madsmtm can you have a look at this PR please?

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated
@@ -2803,6 +2818,8 @@ impl Build {
format!("arm-kmc-eabi-{}", gnu)
} else if target.arch == "aarch64" && target.vendor == "kmc" {
format!("aarch64-kmc-elf-{}", gnu)
} else if target.os == "nto" {
"qcc".into()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like this target only supports compiling with qcc, is that true?

If so, @NobodyXu is this the right place to set this up? This would only select the default compiler, right? What if the user passes cc::Build::new().compiler("clang")? Is the expected behaviour to still try to use Clang, even though we know that'll fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like this target only supports compiling with qcc, is that true?

I think/guess you have to use qcc if you want to include system header files due to license reasons. In other words, usually only qcc is used.
If someone wants to use a very non-default compiler, they can set it via environment variables.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that if we add the -V flag, then you won't be able to compile with other compilers that don't support that flag

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get your point. Thinking about my previous statement: It's not just header files, you also cannot link to create executables or shared objects without license violations. I don't think we need to support non-qcc compilers.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for the clarification.

Gonna keep this open pending answer wrt. default vs. required compiler.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anyone is compiling for QNX Neutrino and not using qcc. If you pay this much for your RTOS, you colour between the lines, not outside.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant, I wanted @NobodyXu to confirm that this was the best place to set it up ;)

@madsmtm
Copy link
Collaborator

madsmtm commented Dec 12, 2024

(Also not familiar with Neutrino QNX, I'm fine with deferring to the target maintainers here)

Copy link
Collaborator

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Final few questions from me:

  • Do we need to do something else to support the io-pkt/io-sock divide? Do we need to pass a special flag or something?
  • Is there a compiler flag for choosing between 7.0, 7.1 and 8.0? E.g. do we actually need to pass e.g. -V7.0,gcc_ntoaarch64le_cxx, or am I misunderstanding that?

@flba-eb
Copy link
Contributor Author

flba-eb commented Dec 12, 2024

Thanks!

Final few questions from me:

  • Do we need to do something else to support the io-pkt/io-sock divide? Do we need to pass a special flag or something?

Only linking requires an additional library search path which is added by rustc when rustc links. As far as I understand cc, it never calls a linker itself. If cc can be used to link shared objects / executables, then it would make sense to investigate.

  • Is there a compiler flag for choosing between 7.0, 7.1 and 8.0? E.g. do we actually need to pass e.g. -V7.0,gcc_ntoaarch64le_cxx, or am I misunderstanding that?

No, I'm only adding -Vgcc_ntoaarch64le_cxx. The version number is optional. Also, this version relates to the used compiler version (e.g. "gcc 8.3"), not the QNX version.

@jonathanpallant
Copy link

The correct compiler is added to your PATH by running a shell script / batch file supplied with the QNX Software Development Platform, so you can't have both QNX SDP 8.0 and QNX SDP 7.1 (for example) versions of qcc in your PATH at the same time.

@jonathanpallant
Copy link

(and the OS is called QNX Neutrino, not Neutrino QNX, fwiw)

@madsmtm
Copy link
Collaborator

madsmtm commented Dec 12, 2024

Only linking requires an additional library search path which is added by rustc when rustc links. As far as I understand cc, it never calls a linker itself. If cc can be used to link shared objects / executables, then it would make sense to investigate.

I think cc does somewhat support this with Build::shared_flag, though I'm not familiar with how well we support it (and I definitely won't block this PR on that).

The correct compiler is added to your PATH by running a shell script / batch file supplied with the QNX Software Development Platform, so you can't have both QNX SDP 8.0 and QNX SDP 7.1 (for example) versions of qcc in your PATH at the same time.

I'm just worried that users might have e.g. QNX SDP 7.1 configured while building the aarch64-unknown-nto-qnx700 target, is there a check against such incompatibilities anywhere?

@flba-eb
Copy link
Contributor Author

flba-eb commented Dec 12, 2024

The correct compiler is added to your PATH by running a shell script / batch file supplied with the QNX Software Development Platform, so you can't have both QNX SDP 8.0 and QNX SDP 7.1 (for example) versions of qcc in your PATH at the same time.

I'm just worried that users might have e.g. QNX SDP 7.1 configured while building the aarch64-unknown-nto-qnx700 target, is there a check against such incompatibilities anywhere?

The toolchain does not reveal the QNX version, unfortunately. You would need to start parsing header files, like I've tried with rust-lang/libc#4192 ... but as also written there:
Even if you are having QNX 7.1 on the target and a QNX 7.1 toolchain, it does not mean they are compatible. Sometimes, values and/or signatures can change in header files between "sub versions" (e.g. which provide special, hardware/project dependent improvements) of e.g. 7.1.

@madsmtm
Copy link
Collaborator

madsmtm commented Dec 12, 2024

Fair, it's unfortunate that that's the state of things, but I guess we don't need to do anything in cc to avoid it then.

src/lib.rs Show resolved Hide resolved
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Dec 13, 2024
…er-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: `@jonathanpallant` `@japaric` `@gh-tr` `@AkhilTThomas`
Zalathar added a commit to Zalathar/rust that referenced this pull request Dec 13, 2024
…er-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: ``@jonathanpallant`` ``@japaric`` ``@gh-tr`` ``@AkhilTThomas``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 13, 2024
…er-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: ```@jonathanpallant``` ```@japaric``` ```@gh-tr``` ```@AkhilTThomas```
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@NobodyXu NobodyXu merged commit 82c8c7b into rust-lang:main Dec 13, 2024
27 checks passed
@github-actions github-actions bot mentioned this pull request Dec 13, 2024
@flba-eb flba-eb deleted the set_qcc branch December 13, 2024 10:28
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 13, 2024
…er-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: ````@jonathanpallant```` ````@japaric```` ````@gh-tr```` ````@AkhilTThomas````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 13, 2024
…er-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: `````@jonathanpallant````` `````@japaric````` `````@gh-tr````` `````@AkhilTThomas`````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 13, 2024
…er-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: ``````@jonathanpallant`````` ``````@japaric`````` ``````@gh-tr`````` ``````@AkhilTThomas``````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 13, 2024
…er-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: ```````@jonathanpallant``````` ```````@japaric``````` ```````@gh-tr``````` ```````@AkhilTThomas```````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 14, 2024
…er-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: ````````@jonathanpallant```````` ````````@japaric```````` ````````@gh-tr```````` ````````@AkhilTThomas````````
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
Rollup merge of rust-lang#134211 - flba-eb:add_qnx_archiver, r=compiler-errors

On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: ````````@jonathanpallant```````` ````````@japaric```````` ````````@gh-tr```````` ````````@AkhilTThomas````````
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Dec 15, 2024
On Neutrino QNX, reduce the need to set archiver via environment variables

This adds support for automatically selecting the correct `ar` tool when compiling for Neutrino QNX.
Once rust-lang/cc-rs#1319 is merged and a new cc version is integrated, all environment variables of the [Neutrino documentation](https://github.com/rust-lang/rust/blob/stable/src/doc/rustc/src/platform-support/nto-qnx.md) can be removed.

CC: ````````@jonathanpallant```````` ````````@japaric```````` ````````@gh-tr```````` ````````@AkhilTThomas````````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants