-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Toggle assert_unsafe_precondition in codegen instead of expansion #120594
Changes from all commits
9842a5c
580067c
55fabf3
8836ac5
61118ff
b0ea682
88d6e9f
9e1b2d9
611c3cb
dbf817b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,12 @@ | |
|
||
use crate::simplify::simplify_duplicate_switch_targets; | ||
use rustc_middle::mir::*; | ||
use rustc_middle::ty::layout; | ||
use rustc_middle::ty::layout::ValidityRequirement; | ||
use rustc_middle::ty::{self, GenericArgsRef, ParamEnv, Ty, TyCtxt}; | ||
use rustc_span::symbol::Symbol; | ||
use rustc_target::abi::FieldIdx; | ||
use rustc_target::spec::abi::Abi; | ||
|
||
pub struct InstSimplify; | ||
|
||
|
@@ -38,6 +40,7 @@ impl<'tcx> MirPass<'tcx> for InstSimplify { | |
block.terminator.as_mut().unwrap(), | ||
&mut block.statements, | ||
); | ||
ctx.simplify_nounwind_call(block.terminator.as_mut().unwrap()); | ||
simplify_duplicate_switch_targets(block.terminator.as_mut().unwrap()); | ||
} | ||
} | ||
|
@@ -252,6 +255,28 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> { | |
terminator.kind = TerminatorKind::Goto { target: destination_block }; | ||
} | ||
|
||
fn simplify_nounwind_call(&self, terminator: &mut Terminator<'tcx>) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is this coming from? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "sometimes"? Strange. Is this a MIR building bug? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know. I tried to fix this in MIR building and it seemed really awkward. I also wonder if inlining will cause the optimization to sometimes see a monomorphic call that was initially built polymorphic. And thus we'd need the opt anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, inlining is a good point. Might be worth a dedicated test. |
||
let TerminatorKind::Call { func, unwind, .. } = &mut terminator.kind else { | ||
return; | ||
}; | ||
|
||
let Some((def_id, _)) = func.const_fn_def() else { | ||
return; | ||
}; | ||
|
||
let body_ty = self.tcx.type_of(def_id).skip_binder(); | ||
let body_abi = match body_ty.kind() { | ||
ty::FnDef(..) => body_ty.fn_sig(self.tcx).abi(), | ||
ty::Closure(..) => Abi::RustCall, | ||
ty::Coroutine(..) => Abi::Rust, | ||
_ => bug!("unexpected body ty: {:?}", body_ty), | ||
}; | ||
|
||
if !layout::fn_can_unwind(self.tcx, Some(def_id), body_abi) { | ||
*unwind = UnwindAction::Unreachable; | ||
} | ||
} | ||
|
||
fn simplify_intrinsic_assert( | ||
&self, | ||
terminator: &mut Terminator<'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.
This calls for careful documentation in
intrinsics.rs
(which is currently missing). If the intrinsic is ever used in cases where there is not immediate UB after a failed assertion, this will lead to a surprising lack of debug checks in Miri (and potentially const-eval).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.
Where should this be documented? In a module doc comment? On the enclosing function?
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 was thinking the doc comment for the
debug_assertions
intrinsic.