Skip to content

Fix x86_64 code being produced for bare-metal LoongArch targets' compiler_builtins #127150

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

xen0n
Copy link
Contributor

@xen0n xen0n commented Jun 30, 2024

Formerly the loongarch*-*-none* targets were added to the dist-various-2 CI job, but no corresponding toolchain was added along with them. This meant the compiler_builtins for the targets were built with the host toolchain.

As the other dist-various toolchains are mostly pre-built so far, to avoid burdening them with crosstool-ng builds, simply move the two bare-metal LoongArch targets to the dist-loongarch64-linux job which has a ready-to-use LoongArch toolchain. With the proper CFLAGS applied it is possible to build artifacts suitable for bare-metal. I verified that the compiler_builtins objects are now correctly produced regarding architecture and ABI, with the changes here applied.

Fixes #125908.

cc @heiher

try-job: dist-loongarch64-linux
try-job: dist-various-2

…piler_builtins`

Formerly the `loongarch*-*-none*` targets were added to the
`dist-various-2` CI job, but no corresponding toolchain was added along
with them. This meant the `compiler_builtins` for the targets were built
with the host toolchain.

As the other `dist-various` toolchains are mostly pre-built so far, to
avoid burdening them with crosstool-ng builds, simply move the two
bare-metal LoongArch targets to the `dist-loongarch64-linux` job which
has a ready-to-use LoongArch toolchain. With the proper CFLAGS applied
it is possible to build artifacts suitable for bare-metal. I verified
that the `compiler_builtins` objects are now correctly produced
regarding architecture and ABI, with the changes here applied.

Fixes rust-lang#125908.

cc @heiher

try-job: dist-loongarch64-linux
try-job: dist-various-2
@rustbot
Copy link
Collaborator

rustbot commented Jun 30, 2024

r? @Kobzol

rustbot has assigned @Kobzol.
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 A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jun 30, 2024
@xen0n
Copy link
Contributor Author

xen0n commented Jun 30, 2024

(Again) I'll need someone to trigger a try build for the PR, thanks in advance!

@Kobzol
Copy link
Contributor

Kobzol commented Jun 30, 2024

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 30, 2024
Fix x86_64 code being produced for bare-metal LoongArch targets' `compiler_builtins`

Formerly the `loongarch*-*-none*` targets were added to the `dist-various-2` CI job, but no corresponding toolchain was added along with them. This meant the `compiler_builtins` for the targets were built with the host toolchain.

As the other `dist-various` toolchains are mostly pre-built so far, to avoid burdening them with crosstool-ng builds, simply move the two bare-metal LoongArch targets to the `dist-loongarch64-linux` job which has a ready-to-use LoongArch toolchain. With the proper CFLAGS applied it is possible to build artifacts suitable for bare-metal. I verified that the `compiler_builtins` objects are now correctly produced regarding architecture and ABI, with the changes here applied.

Fixes rust-lang#125908.

cc `@heiher`

try-job: dist-loongarch64-linux
try-job: dist-various-2
@bors
Copy link
Collaborator

bors commented Jun 30, 2024

⌛ Trying commit 03fce36 with merge dddb502...

@bors
Copy link
Collaborator

bors commented Jun 30, 2024

☀️ Try build successful - checks-actions
Build commit: dddb502 (dddb502f8087003da5349bdf25243f63c1865e04)

@xen0n
Copy link
Contributor Author

xen0n commented Jun 30, 2024

The CI artifacts are correct as well:

libcompiler_builtins-01ecf15d7725de80.rlib(45c91108d938afe8-muldc3.o):  file format elf64-loongarch

Disassembly of section .text.__muldc3:

0000000000000000 <__muldc3>:
       0: 27 0c 05 01   fmul.d  $fa7, $fa1, $fa3
       4: 06 08 05 01   fmul.d  $fa6, $fa0, $fa2
       8: 09 0c 05 01   fmul.d  $ft1, $fa0, $fa3
       c: 48 04 05 01   fmul.d  $ft0, $fa2, $fa1
...
libcompiler_builtins-470063ddd93ab668.rlib(45c91108d938afe8-muldc3.o):  file format elf64-loongarch

Disassembly of section .text.__muldc3:

0000000000000000 <__muldc3>:
       0: 63 00 fe 02   addi.d  $sp, $sp, -128
       4: 77 a0 c1 29   st.d    $s0, $sp, 104
       8: b7 00 15 00   move    $s0, $a1
       c: c5 00 15 00   move    $a1, $a2
...

The ABI is again correct. I've checked all the other rlibs too and all objects are properly LoongArch.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 30, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 30, 2024

📌 Commit 03fce36 has been approved by Kobzol

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 30, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 1, 2024
Fix x86_64 code being produced for bare-metal LoongArch targets' `compiler_builtins`

Formerly the `loongarch*-*-none*` targets were added to the `dist-various-2` CI job, but no corresponding toolchain was added along with them. This meant the `compiler_builtins` for the targets were built with the host toolchain.

As the other `dist-various` toolchains are mostly pre-built so far, to avoid burdening them with crosstool-ng builds, simply move the two bare-metal LoongArch targets to the `dist-loongarch64-linux` job which has a ready-to-use LoongArch toolchain. With the proper CFLAGS applied it is possible to build artifacts suitable for bare-metal. I verified that the `compiler_builtins` objects are now correctly produced regarding architecture and ABI, with the changes here applied.

Fixes rust-lang#125908.

cc `@heiher`

try-job: dist-loongarch64-linux
try-job: dist-various-2
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 1, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126923 (test: dont optimize to invalid bitcasts)
 - rust-lang#127090 (Reduce merge conflicts from rustfmt's wrapping)
 - rust-lang#127105 (Only update `Eq` operands in GVN if it can update both sides)
 - rust-lang#127150 (Fix x86_64 code being produced for bare-metal LoongArch targets' `compiler_builtins`)
 - rust-lang#127181 (Introduce a `rustc_` attribute to dump all the `DefId` parents of a `DefId`)
 - rust-lang#127182 (Fix error in documentation for IpAddr::to_canonical and Ipv6Addr::to_canonical)
 - rust-lang#127191 (Ensure `out_of_scope_macro_calls` lint is registered)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c7e64dc into rust-lang:master Jul 1, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 1, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 1, 2024
Rollup merge of rust-lang#127150 - xen0n:issue125908, r=Kobzol

Fix x86_64 code being produced for bare-metal LoongArch targets' `compiler_builtins`

Formerly the `loongarch*-*-none*` targets were added to the `dist-various-2` CI job, but no corresponding toolchain was added along with them. This meant the `compiler_builtins` for the targets were built with the host toolchain.

As the other `dist-various` toolchains are mostly pre-built so far, to avoid burdening them with crosstool-ng builds, simply move the two bare-metal LoongArch targets to the `dist-loongarch64-linux` job which has a ready-to-use LoongArch toolchain. With the proper CFLAGS applied it is possible to build artifacts suitable for bare-metal. I verified that the `compiler_builtins` objects are now correctly produced regarding architecture and ABI, with the changes here applied.

Fixes rust-lang#125908.

cc ``@heiher``

try-job: dist-loongarch64-linux
try-job: dist-various-2
@xen0n xen0n deleted the issue125908 branch July 1, 2024 12:03
@workingjubilee
Copy link
Member

Thanks for cleaning this up!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid compiler_builtins produced for loongarch64-unknown-none
5 participants