-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Refactor rustc_codegen_ssa #56198
Refactor rustc_codegen_ssa #56198
Conversation
@@ -98,7 +98,6 @@ impl BackendTypes for CodegenCx<'ll, 'tcx> { | |||
type Value = &'ll Value; | |||
type BasicBlock = &'ll BasicBlock; | |||
type Type = &'ll Type; | |||
type Context = &'ll llvm::Context; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoa, this is nice!
@@ -763,6 +763,95 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { | |||
} | |||
} | |||
} | |||
|
|||
fn abort(&mut self) { | |||
let fnname = self.cx().get_intrinsic(&("llvm.trap")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sunfishcode I think this builder method should be called trap
, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In cranelift the Interrupt
trap can be resumed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, Cranelift should add an explicit way to request an "abort" trap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what should we call this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how bx.abort() lines up with mir::TerminatorKind::Abort. But either way works for me.
#[derive(Copy, Clone)] | ||
pub enum OverflowOp { | ||
Add, Sub, Mul | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@denismerigoux put some enums in common.rs
but I'm not sure, this might be better.
@bors r+ |
📌 Commit 36baa97 has been approved by |
src/librustc_mir/interpret/traits.rs
Outdated
Usize(u64), | ||
Nullptr, | ||
FnPtr(Instance<'tcx>), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this should be done without an EvalContext
. With my recent changes to Allocation
s this should be mostly possible. I was hoping on getting https://github.com/rust-lang/rust/pull/55392/files#diff-2eb1005fe45cbe73c0ddfcde017b5f73R83 into master before having to address vtable generation, because that function would make such an operation trivial. You can still look at that function as a reference for creating the function that generates a vtable Allocation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code should probably move to rustc::mir::interpret::traits
, too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am backing out the vtable generation changes for now, because they are a bit too much work for me right now.
8a67d20
to
cd66200
Compare
cd66200
to
1626e59
Compare
1626e59
to
0fc95f9
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
0fc95f9
to
d108a91
Compare
@bors r+ |
📌 Commit d108a91 has been approved by |
Refactor rustc_codegen_ssa cc #56108 (not all things are done yet) This removes an unsafe method from cg_ssa. r? @eddyb cc @sunfishcode
☀️ Test successful - status-appveyor, status-travis |
cc #56108 (not all things are done yet)
This removes an unsafe method from cg_ssa.
r? @eddyb
cc @sunfishcode