Unwinding support for inline assembly #88439

cynecx · 2021-08-28T21:31:38Z

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

compiler/rustc_ast_pretty/src/pprust/state.rs

compiler/rustc_codegen_cranelift/src/base.rs

compiler/rustc_codegen_llvm/src/asm.rs

Amanieu · 2021-08-30T00:36:39Z

I actually started working on a PR myself before you posted this one, but it's still very much a WIP: master...Amanieu:asm_unwind

More changes to MIR are needed to support this since MIR needs to be aware of unwinding. In particular call_return_effect needs to be adapted to also work with TerminatorKind::InlineAsm. This is used to mark the return value of a call as initialized only if a call returns normally but not when it unwinds. The same logic applies for output operands of InlineAsm.

compiler/rustc_codegen_llvm/src/asm.rs

Amanieu · 2021-08-30T21:05:49Z

I updated my branch at master...Amanieu:asm_unwind, it should now handle all of the MIR parts that deal with unwinding.

cynecx · 2021-08-30T21:57:50Z

@Amanieu That's great! Do you mind if I rebase this PR onto your branch? (at the end just adding codegen)

Amanieu · 2021-08-30T21:58:42Z

Sure! Just keep in mind I haven't actually tested any of it apart from checking it compiles.

cynecx · 2021-08-30T22:01:44Z

@Amanieu No worries. I am going to add some tests anyway and so then we shall see :) (But that has to wait until tomorrow or the day after tomorrow. But codgen-wise, it should be correct, as I've seen here: cynecx/libfringe#1 (comment). EDIT: however there might have been some issues after all because I haven't properly dealt with MIR xD).

Amanieu · 2021-08-31T23:24:21Z

I updated my branch again, there were a few more things I missed in MIR.

Amanieu

Can you also:

update the documentation in the unstable book to add may_unwind.
double-check the interaction between #[naked] and may_unwind: does this allow you to unwind from a naked function?

compiler/rustc_codegen_cranelift/src/base.rs

cynecx · 2021-09-04T19:44:57Z

update the documentation in the unstable book to add may_unwind.

Will do.

double-check the interaction between #[naked] and may_unwind: does this allow you to unwind from a naked function?

As far as I have seen from several llvm-ir/assembly dumps (irl testcase: [1]), it should work just fine.

[1] https://github.com/cynecx/libfringe/blob/asm/src/arch/x86_64.rs (your unwinding branch for libfringe basically)

bors · 2021-09-09T00:21:13Z

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

Amanieu · 2021-09-18T20:02:18Z

@cynecx Are you still working on this PR?

cynecx · 2021-09-19T18:11:54Z

@Amanieu Yes I am. Sorry about the delay, I was caught up in some other things... Trying a rebase rn.

Amanieu · 2021-09-22T16:12:32Z

@cynecx are there any other changes that you plan on making (this PR is still marked as a draft)?

Otherwise this LGTM. I'd like someone more familiar with MIR to review the MIR-level changes here.

r? @oli-obk

bors · 2021-12-02T22:13:41Z

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

cynecx · 2021-12-03T23:37:57Z

@Amanieu rebased and test fixed.

Amanieu · 2021-12-03T23:47:37Z

@bors r+

bors · 2021-12-03T23:47:39Z

📌 Commit 3dbb621 has been approved by Amanieu

bors · 2021-12-04T05:59:20Z

⌛ Testing commit 3dbb621 with merge 887999d...

bors · 2021-12-04T09:05:27Z

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 887999d to master...

rust-timer · 2021-12-04T10:35:46Z

Finished benchmarking commit (887999d): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

rust-highfive assigned Amanieu Aug 28, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 28, 2021