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.

Sign up for GitHub

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

Jump to bottom

remove llvm.assertions=true in compiler profile #121476

Merged
merged 1 commit into from
Feb 23, 2024
Merged

remove llvm.assertions=true in compiler profile #121476

merged 1 commit into from
Feb 23, 2024

Conversation

onur-ozkan
Copy link
Member

Having this set to true disrupts compiler development workflows for people who use llvm.download-ci-llvm = true because we don't provide ci-llvm on the rustc-alt-builds server. Therefore, it is kept off by default.

cc @Nilstrieb @compiler-errors

For more context, see https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/CI.20LLVM.20for.20aarch64

@rustbot
Copy link
Collaborator

rustbot commented Feb 22, 2024

r? @clubby789

rustbot has assigned @clubby789.
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 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) labels Feb 22, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 22, 2024

This PR modifies src/bootstrap/defaults.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

compiler-errors
compiler-errors approved these changes Feb 22, 2024
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

thanks, appreciate you investigating this

@onur-ozkan
Copy link
Member Author

This PR modifies src/bootstrap/defaults.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

No need for an additional change-tracker entry as this was just recently added and not relied upon by many people anyway.

@onur-ozkan
set llvm.assertions to false in compiler profile
52227ed 
Having this set to true disrupts compiler development workflows for people who use `llvm.download-ci-llvm = true`
because we don't provide ci-llvm on the `rustc-alt-builds` server. Therefore, it is kept off by default.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan force-pushed the update-compiler-profile branch from 0470ad3 to 52227ed Compare February 22, 2024 19:15
@Noratrieb
Copy link
Member

:( sad, need more alt builds

@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 22, 2024

📌 Commit 52227ed has been approved by compiler-errors

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 Feb 22, 2024
@Zalathar
Copy link
Contributor

It's unfortunate that this silently disables LLVM assertions for people who were previously using the codegen profile, and only just migrated over to the compiler profile.

At the very least, it might be a good idea to add a note to the top of #121278 telling codegen profile users to explicitly enable LLVM assertions in their own config.toml.

@matthiaskrgr matthiaskrgr mentioned this pull request Feb 23, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2024
@bors
Auto merge of rust-lang#121491 - matthiaskrgr:rollup-wkzqawy, r=matth…
fdf8b29 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121434 (Fix rust-lang#121208 fallout)
 - rust-lang#121471 (When encountering `<&T as Clone>::clone(x)` because `T: Clone`, suggest `#[derive(Clone)]`)
 - rust-lang#121476 (remove `llvm.assertions=true` in compiler profile)
 - rust-lang#121479 (fix generalizer unsoundness)
 - rust-lang#121480 (Fix more rust-lang#121208 fallout)
 - rust-lang#121482 (Allow for a missing `adt_def` in `NamePrivacyVisitor`.)
 - rust-lang#121484 (coverage: Use variable name `this` in `CoverageGraph::from_mir`)
 - rust-lang#121487 (Explicitly call `emit_stashed_diagnostics`.)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2024
@bors
Auto merge of rust-lang#121491 - matthiaskrgr:rollup-wkzqawy, r=matth…
52cea08 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121434 (Fix rust-lang#121208 fallout)
 - rust-lang#121471 (When encountering `<&T as Clone>::clone(x)` because `T: Clone`, suggest `#[derive(Clone)]`)
 - rust-lang#121476 (remove `llvm.assertions=true` in compiler profile)
 - rust-lang#121479 (fix generalizer unsoundness)
 - rust-lang#121480 (Fix more rust-lang#121208 fallout)
 - rust-lang#121482 (Allow for a missing `adt_def` in `NamePrivacyVisitor`.)
 - rust-lang#121484 (coverage: Use variable name `this` in `CoverageGraph::from_mir`)
 - rust-lang#121487 (Explicitly call `emit_stashed_diagnostics`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 977bbb4 into rust-lang:master Feb 23, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2024
@rust-timer
Unrolled build for rust-lang#121476
adb6956 
Rollup merge of rust-lang#121476 - onur-ozkan:update-compiler-profile, r=compiler-errors

remove `llvm.assertions=true` in compiler profile

Having this set to true disrupts compiler development workflows for people who use `llvm.download-ci-llvm = true` because we don't provide ci-llvm on the `rustc-alt-builds` server. Therefore, it is kept off by default.

cc `@Nilstrieb` `@compiler-errors`

For more context, see https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/CI.20LLVM.20for.20aarch64
@onur-ozkan onur-ozkan deleted the update-compiler-profile branch February 23, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@clubby789 clubby789

Labels
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)
Projects
None yet
Milestone
1.78.0
Development

Successfully merging this pull request may close these issues.
7 participants
@onur-ozkan @rustbot @Noratrieb @compiler-errors @bors @Zalathar @clubby789