Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF` #109373

ids1024 · 2023-03-20T03:15:21Z

This option was added to LLVM in https://reviews.llvm.org/D121750?id=416339. It makes llvm_unreachable in builds without assertions compile to an LLVM_BUILTIN_TRAP instead of LLVM_BUILTIN_UNREACHABLE (which causes undefined behavior and is equivalent to std::hint::unreachable_unchecked).

Having compiler bugs triggering undefined behavior generally seems undesirable and inconsistent with Rust's goals. There is a check in src/tools/tidy/src/style.rs to reject code using llvm_unreachable. But it is used a lot within LLVM itself.

For instance, this changes a failure I get compiling libcore for m68k from a SIGSEGV to SIGILL, which seems better though it still doesn't provide a useful message without switching to an LLVM build with asserts.

It may be best not to do this if it noticeably degrades compiler performance, but worthwhile if it doesn't do so in any significant way. I haven't looked into what benchmarks there are for Rustc. That should be considered before merging.

rustbot · 2023-03-20T03:15:30Z

r? @ozkanonur

(rustbot has picked a reviewer for you, use r? to override)

ids1024 · 2023-03-20T03:27:35Z

I thought about opening an issue for this, but might as well just be a PR since it only takes one line to change the option. Something Rust should consider at least.

Noratrieb · 2023-03-20T06:03:03Z

@bors r+ @rust-timer queue

Noratrieb · 2023-03-20T06:03:10Z

@bors r-

Noratrieb · 2023-03-20T06:03:20Z

oops, wrong command
@bors try

bors · 2023-03-20T06:03:29Z

⌛ Trying commit a6c3e0c0283908ba2433e6e52882ac114a96f961 with merge 1dc0c6ee15d57fe1b7fda23cd4e4711a65b4d96f...

Noratrieb · 2023-03-20T06:35:12Z

Actually, I don't think our LLVM submodule has this new change yet, does it?

onur-ozkan · 2023-03-20T06:47:27Z

Actually, I don't think our LLVM submodule has this new change yet, does it?

It does have.

rust-lang/llvm-project@7b98391

bors · 2023-03-20T08:25:36Z

☀️ Try build successful - checks-actions
Build commit: 1dc0c6ee15d57fe1b7fda23cd4e4711a65b4d96f (1dc0c6ee15d57fe1b7fda23cd4e4711a65b4d96f)

bors · 2023-03-20T08:25:36Z

☀️ Try build successful - checks-actions
Build commit: 1dc0c6ee15d57fe1b7fda23cd4e4711a65b4d96f (1dc0c6ee15d57fe1b7fda23cd4e4711a65b4d96f)

rust-timer · 2023-03-20T11:30:19Z

Finished benchmarking commit (1dc0c6ee15d57fe1b7fda23cd4e4711a65b4d96f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

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.7%	[0.6%, 2.8%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-1.2%, -0.5%]	3
Improvements ✅ (secondary)	-3.4%	[-4.7%, -2.1%]	2
All ❌✅ (primary)	0.1%	[-1.2%, 2.8%]	5

Cycles

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)	0.5%	[0.5%, 0.5%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.6%	[-0.6%, -0.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.0%	[-0.6%, 0.5%]	2

onur-ozkan

@bors r+

onur-ozkan · 2023-03-21T09:02:52Z

ops, bors can't get triggered from review note seems like

@bors r+

bors · 2023-03-21T09:02:54Z

📌 Commit a6c3e0c0283908ba2433e6e52882ac114a96f961 has been approved by ozkanonur

It is now in the queue for this repository.

Noratrieb · 2023-03-21T10:01:05Z

perf was neutral and the queue is pretty filled up
@bors rollup=maybe
feel free to set it to rollup=never again if we prefer to not roll up changes like this (I'm not sure on that)

onur-ozkan · 2023-03-21T10:08:24Z

feel free to set it to rollup=never again if we prefer to not roll up changes like this (I'm not sure on that)

I forgot to do that, thanks

src/bootstrap/native.rs

onur-ozkan · 2023-03-21T11:17:27Z

@bors r-

This option was added to LLVM in https://reviews.llvm.org/D121750?id=416339. It makes `llvm_unreachable` in builds without assertions compile to an `LLVM_BUILTIN_TRAP` instead of `LLVM_BUILTIN_UNREACHABLE` (which causes undefined behavior and is equivalent to `std::hint::unreachable_unchecked`). Having compiler bugs triggering undefined behavior generally seems undesirable and inconsistent with Rust's goals. There is a check in `src/tools/tidy/src/style.rs` to reject code using `llvm_unreachable`. But it is used a lot within LLVM itself. For instance, this changes a failure I get compiling `libcore` for m68k from a `SIGSEGV` to `SIGILL`, which seems better though it still doesn't provide a useful message without switching to an LLVM build with asserts. It may be best not to do this if it noticeably degrades compiler performance, but worthwhile if it doesn't do so in any significant way. I haven't looked into what benchmarks there are for Rustc. That should be considered before merging.

onur-ozkan · 2023-03-21T15:33:09Z

@bors r+

bors · 2023-03-21T15:33:11Z

📌 Commit dfbf610 has been approved by ozkanonur

It is now in the queue for this repository.

onur-ozkan · 2023-03-21T15:33:48Z

@bors rollup

… r=ozkanonur Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF` This option was added to LLVM in https://reviews.llvm.org/D121750?id=416339. It makes `llvm_unreachable` in builds without assertions compile to an `LLVM_BUILTIN_TRAP` instead of `LLVM_BUILTIN_UNREACHABLE` (which causes undefined behavior and is equivalent to `std::hint::unreachable_unchecked`). Having compiler bugs triggering undefined behavior generally seems undesirable and inconsistent with Rust's goals. There is a check in `src/tools/tidy/src/style.rs` to reject code using `llvm_unreachable`. But it is used a lot within LLVM itself. For instance, this changes a failure I get compiling `libcore` for m68k from a `SIGSEGV` to `SIGILL`, which seems better though it still doesn't provide a useful message without switching to an LLVM build with asserts. It may be best not to do this if it noticeably degrades compiler performance, but worthwhile if it doesn't do so in any significant way. I haven't looked into what benchmarks there are for Rustc. That should be considered before merging.

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#109373 (Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF`) - rust-lang#109392 (Custom MIR: Allow optional RET type annotation) - rust-lang#109394 (adapt tests/codegen/vec-shrink-panik for LLVM 17) - rust-lang#109412 (rustdoc: Add GUI test for "Auto-hide item contents for large items" setting) - rust-lang#109452 (Ignore the vendor directory for tidy tests.) - rust-lang#109457 (Remove comment about reusing rib allocations) - rust-lang#109461 (rustdoc: remove redundant `.content` prefix from span/a colors) - rust-lang#109477 (`HirId` to `LocalDefId` cleanup) - rust-lang#109489 (More general captures) - rust-lang#109494 (Do not feed param_env for RPITITs impl side) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned onur-ozkan Mar 20, 2023

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 Mar 20, 2023

This comment has been minimized.

Sign in to view

This comment was marked as resolved.

Sign in to view

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 Mar 20, 2023

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 20, 2023

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 20, 2023

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 20, 2023

onur-ozkan approved these changes Mar 21, 2023

View reviewed changes

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 21, 2023

bjorn3 reviewed Mar 21, 2023

View reviewed changes

src/bootstrap/native.rs Show resolved Hide resolved

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 21, 2023

ids1024 force-pushed the llvm-unreachable-optimize branch from a6c3e0c to dfbf610 Compare March 21, 2023 14:58

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 21, 2023

Dylan-DPC mentioned this pull request Mar 21, 2023

Rollup of 8 pull requests #109450

Closed

Dylan-DPC mentioned this pull request Mar 22, 2023

Rollup of 12 pull requests #109490

Closed

matthiaskrgr mentioned this pull request Mar 22, 2023

Rollup of 10 pull requests #109497

Merged

bors merged commit 56959e5 into rust-lang:master Mar 23, 2023

rustbot added this to the 1.70.0 milestone Mar 23, 2023

MasterAwesome mentioned this pull request Oct 4, 2023

Cannot build obj.clangAST when LLVM_UNREACHABLE_OPTIMIZATIONS are turned off llvm/llvm-project#68237

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF` #109373

Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF` #109373

ids1024 commented Mar 20, 2023

rustbot commented Mar 20, 2023

ids1024 commented Mar 20, 2023

Noratrieb commented Mar 20, 2023

This comment has been minimized.

This comment was marked as resolved.

Noratrieb commented Mar 20, 2023

Noratrieb commented Mar 20, 2023

bors commented Mar 20, 2023

Noratrieb commented Mar 20, 2023

onur-ozkan commented Mar 20, 2023 •

edited

Loading

bors commented Mar 20, 2023

bors commented Mar 20, 2023

This comment has been minimized.

rust-timer commented Mar 20, 2023

onur-ozkan left a comment

onur-ozkan commented Mar 21, 2023

bors commented Mar 21, 2023

Noratrieb commented Mar 21, 2023

onur-ozkan commented Mar 21, 2023

onur-ozkan commented Mar 21, 2023

onur-ozkan commented Mar 21, 2023

bors commented Mar 21, 2023

onur-ozkan commented Mar 21, 2023

Set LLVM LLVM_UNREACHABLE_OPTIMIZE to OFF #109373

Set LLVM LLVM_UNREACHABLE_OPTIMIZE to OFF #109373

Conversation

ids1024 commented Mar 20, 2023

rustbot commented Mar 20, 2023

ids1024 commented Mar 20, 2023

Noratrieb commented Mar 20, 2023

This comment has been minimized.

This comment was marked as resolved.

Noratrieb commented Mar 20, 2023

Noratrieb commented Mar 20, 2023

bors commented Mar 20, 2023

Noratrieb commented Mar 20, 2023

onur-ozkan commented Mar 20, 2023 • edited Loading

bors commented Mar 20, 2023

bors commented Mar 20, 2023

This comment has been minimized.

rust-timer commented Mar 20, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

onur-ozkan left a comment

Choose a reason for hiding this comment

onur-ozkan commented Mar 21, 2023

bors commented Mar 21, 2023

Noratrieb commented Mar 21, 2023

onur-ozkan commented Mar 21, 2023

onur-ozkan commented Mar 21, 2023

onur-ozkan commented Mar 21, 2023

bors commented Mar 21, 2023

onur-ozkan commented Mar 21, 2023

Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF` #109373

Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF` #109373

onur-ozkan commented Mar 20, 2023 •

edited

Loading