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

asm: add support for noreturn option #717

Merged
merged 5 commits into from
Aug 13, 2021

Conversation

msiglreith
Copy link
Contributor

OpUnreachable will be appended as terminator at the end of the asm block (done automatically by the rustc)

Fixes #570

OpUnreachable will be appended as terminator at the end of the asm block.
Copy link
Contributor

@khyperia khyperia left a comment

Choose a reason for hiding this comment

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

With options(noreturn), the user should not have to write %unused = OpLabel, which is quite a horrible workaround hack.

I believe the proper fix here is to add full understanding of basic blocks to the asm emitter, rather than shoving basic block headers/terminators into blocks[i].instructions.

Specifically, the emitter should keep track of whether the current context is within a basic block, keep track of what that block is (if there is a block), validate that no invalidly placed instructions are present (e.g. OpAdd after a terminator), and assert that at the end of the block, if noreturn is not present, there is an active block, but if noreturn is present, there is not an active block.

@msiglreith
Copy link
Contributor Author

Specifically, the emitter should keep track of whether the current context is within a basic block, keep track of what that block is (if there is a block), validate that no invalidly placed instructions are present (e.g. OpAdd after a terminator), and assert that at the end of the block, if noreturn is not present, there is an active block, but if noreturn is present, there is not an active block.

I tried to incorporate the suggested approach. Please have a look again!

@@ -328,10 +366,32 @@ impl<'cx, 'tcx> Builder<'cx, 'tcx> {
}
return;
}
_ => {

op => {
self.emit()
.insert_into_block(dr::InsertPoint::End, inst)
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Still would be nice to not shove everything into the same block, including terminators and labels, but whatever I guess!

@khyperia khyperia merged commit acda771 into EmbarkStudios:main Aug 13, 2021
@msiglreith msiglreith deleted the asm-noreturn branch August 26, 2021 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement options(noreturn) for asm!.
2 participants