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

Run part of rustc_codegen_gcc's tests in CI #117313

Merged
merged 19 commits into from
Nov 3, 2023

Conversation

GuillaumeGomez
Copy link
Member

Thanks to #112701 and @bjorn3, it made this much easier.

Also cc @antoyo.

r? @bjorn3

@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-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-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Oct 28, 2023
@rustbot
Copy link
Collaborator

rustbot commented Oct 28, 2023

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

@rust-log-analyzer

This comment has been minimized.

src/ci/run.sh Outdated Show resolved Hide resolved
@bjorn3
Copy link
Member

bjorn3 commented Oct 28, 2023

At least the x86_64-gnu-llvm-15 runner doesn't have libgccjit installed.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member

bjorn3 commented Oct 28, 2023

2023-10-28T16:54:08.4668438Z   = note: /usr/bin/ld: /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/arbitrary_self_types_pointers_and_wrappers.arbitrary_self_types_pointers_and_wrappers.4f746435a173466d-cgu.0.rcgu.o: in function `<i32 as core::fmt::Debug>::fmt':
2023-10-28T16:54:08.4669301Z           fake.c:(.text._RNvXs1g_NtNtCseVSX39tBz28_4core3fmt3numlNtB8_5Debug3fmtCs6OVZ8sMh4wz_42arbitrary_self_types_pointers_and_wrappers+0x5f): undefined reference to `core::fmt::num::__LT_impl_u20_core__fmt__LowerHex_u20_for_u20_i32_GT_::fmt'
2023-10-28T16:54:08.4670212Z           /usr/bin/ld: fake.c:(.text._RNvXs1g_NtNtCseVSX39tBz28_4core3fmt3numlNtB8_5Debug3fmtCs6OVZ8sMh4wz_42arbitrary_self_types_pointers_and_wrappers+0x81): undefined reference to `core::fmt::num::imp::__LT_impl_u20_core__fmt__Display_u20_for_u20_i32_GT_::fmt'
2023-10-28T16:54:08.4671109Z           /usr/bin/ld: fake.c:(.text._RNvXs1g_NtNtCseVSX39tBz28_4core3fmt3numlNtB8_5Debug3fmtCs6OVZ8sMh4wz_42arbitrary_self_types_pointers_and_wrappers+0xa3): undefined reference to `core::fmt::num::__LT_impl_u20_core__fmt__UpperHex_u20_for_u20_i32_GT_::fmt'
2023-10-28T16:54:08.4671270Z           collect2: error: ld returned 1 exit status

I presume it is trying to use a standard library compiled without -Csymbol-mangling-version=v0? By the way should cg_gcc maybe error out on -Csymbol-mangling-version=legacy?

@GuillaumeGomez
Copy link
Member Author

I presume it is trying to use a standard library compiled without -Csymbol-mangling-version=v0?

Yes, but I don't understand how it's possible. The RUSTC_SYSROOT env variable is set and the -Zcodegen-backend=gcc flag is passed so I suppose it'll use this one. Maybe I'm missing something.

By the way should cg_gcc maybe error out on -Csymbol-mangling-version=legacy?

I'll let @antoyo decide on that.

@bors
Copy link
Contributor

bors commented Oct 28, 2023

☔ The latest upstream changes (presumably #81746) made this pull request unmergeable. Please resolve the merge conflicts.

@antoyo
Copy link
Contributor

antoyo commented Oct 28, 2023

By the way should cg_gcc maybe error out on -Csymbol-mangling-version=legacy?

I'll let @antoyo decide on that.

I don't mind keeping it available since it partly works.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the cg_gcc-tests branch 2 times, most recently from 66a290b to 9fe0228 Compare November 2, 2023 21:38
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

cg_gcc tests passed again by modifying where we set the environment variables so time to give it another try.

@bors r=onur-ozkan

@bors
Copy link
Contributor

bors commented Nov 3, 2023

📌 Commit c890dd6 has been approved by onur-ozkan

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 Nov 3, 2023
@bors
Copy link
Contributor

bors commented Nov 3, 2023

⌛ Testing commit c890dd6 with merge 6b9d6de...

@bors
Copy link
Contributor

bors commented Nov 3, 2023

☀️ Test successful - checks-actions
Approved by: onur-ozkan
Pushing 6b9d6de to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 3, 2023
@bors bors merged commit 6b9d6de into rust-lang:master Nov 3, 2023
12 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Nov 3, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6b9d6de): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

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.3% [0.3%, 0.3%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.2%] 1
All ❌✅ (primary) 0.3% [0.3%, 0.3%] 1

Max RSS (memory usage)

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)
1.1% [1.1%, 1.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-4.7% [-4.7%, -4.7%] 1
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) -1.8% [-4.7%, 1.1%] 2

Cycles

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

Binary size

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

Bootstrap: 636.745s -> 636.637s (-0.02%)
Artifact size: 304.48 MiB -> 304.47 MiB (-0.00%)

bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Nov 3, 2023
82: Automated pull from upstream `master` r=tshepang a=github-actions[bot]


This PR pulls the following changes from the upstream repository:

* rust-lang/rust#117313
* rust-lang/rust#117131
* rust-lang/rust#117134
* rust-lang/rust#117471
* rust-lang/rust#117521
* rust-lang/rust#117513
  * rust-lang/rust#117512
  * rust-lang/rust#117509
  * rust-lang/rust#117495
  * rust-lang/rust#117394
* rust-lang/rust#117466
* rust-lang/rust#117204
* rust-lang/rust#117386
* rust-lang/rust#117506



Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com>
Co-authored-by: roblabla <unfiltered@roblab.la>
Co-authored-by: Michael Goulet <michael@errs.io>
Co-authored-by: massivebird <gdrakemail@gmail.com>
Co-authored-by: bors <bors@rust-lang.org>
Co-authored-by: Zalathar <Zalathar@users.noreply.github.com>
Co-authored-by: lcnr <rust@lcnr.de>
Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
Co-authored-by: Matthias Krüger <matthias.krueger@famsik.de>
@GuillaumeGomez GuillaumeGomez deleted the cg_gcc-tests branch November 3, 2023 09:52
@GuillaumeGomez
Copy link
Member Author

It finally passed! \o/

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 7, 2023
…,GuillaumeGomez

Re-enable `rustc_codegen_gcc` tests in CI

When rust-lang#117947 dropped llvm-15 from CI, we neglected to copy rust-lang#117313's changes to enable `rustc_codegen_gcc` testing to the new base llvm-16. This is now restored, as well as copying the setup to llvm-17 as well so we hopefully won't miss it next time.

In addition, due to case mismatch in `$extra_env` updates in `docker/run.sh`, I think it wasn't actually getting enabled before, but this should now be fixed. I also avoided the linker hack for `libgccjit.so` that was present before, because that's not needed if the version matches the base `gcc` used for linking.

r? GuillaumeGomez
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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-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-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.