Skip to content

Commit

Permalink
compiler: Never debug_assert in codegen
Browse files Browse the repository at this point in the history
The gains in performance are not worth the costs in correctness.
This is partly because the gains are zero and the costs are unknown.
  • Loading branch information
workingjubilee committed Jul 20, 2024
1 parent 9057c3f commit ce7b069
Show file tree
Hide file tree
Showing 16 changed files with 57 additions and 65 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ impl<'ll> CodegenCx<'ll, '_> {

// If this assertion triggers, there's something wrong with commandline
// argument validation.
debug_assert!(
assert!(
!(self.tcx.sess.opts.cg.linker_plugin_lto.enabled()
&& self.tcx.sess.target.is_like_windows
&& self.tcx.sess.opts.cg.prefer_dynamic)
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
) -> DINodeCreationResult<'ll> {
// The debuginfo generated by this function is only valid if `ptr_type` is really just
// a (fat) pointer. Make sure it is not called for e.g. `Box<T, NonZSTAllocator>`.
debug_assert_eq!(
assert_eq!(
cx.size_and_align_of(ptr_type),
cx.size_and_align_of(Ty::new_mut_ptr(cx.tcx, pointee_type))
);
Expand All @@ -185,7 +185,7 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
match fat_pointer_kind(cx, pointee_type) {
None => {
// This is a thin pointer. Create a regular pointer type and give it the correct name.
debug_assert_eq!(
assert_eq!(
(data_layout.pointer_size, data_layout.pointer_align.abi),
cx.size_and_align_of(ptr_type),
"ptr_type={ptr_type}, pointee_type={pointee_type}",
Expand Down Expand Up @@ -240,8 +240,8 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
FatPtrKind::Slice => ("data_ptr", "length"),
};

debug_assert_eq!(abi::FAT_PTR_ADDR, 0);
debug_assert_eq!(abi::FAT_PTR_EXTRA, 1);
assert_eq!(abi::FAT_PTR_ADDR, 0);
assert_eq!(abi::FAT_PTR_EXTRA, 1);

// The data pointer type is a regular, thin pointer, regardless of whether this
// is a slice or a trait object.
Expand Down Expand Up @@ -498,7 +498,7 @@ pub fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll D
}
};

debug_assert_eq!(di_node_for_uid as *const _, di_node as *const _);
assert_eq!(di_node_for_uid as *const _, di_node as *const _);
} else {
debug_context(cx).type_map.insert(unique_type_id, di_node);
}
Expand Down Expand Up @@ -1060,7 +1060,7 @@ fn build_struct_type_di_node<'ll, 'tcx>(
let ty::Adt(adt_def, _) = struct_type.kind() else {
bug!("build_struct_type_di_node() called with non-struct-type: {:?}", struct_type);
};
debug_assert!(adt_def.is_struct());
assert!(adt_def.is_struct());
let containing_scope = get_namespace_for_item(cx, adt_def.did());
let struct_type_and_layout = cx.layout_of(struct_type);
let variant_def = adt_def.non_enum_variant();
Expand Down Expand Up @@ -1130,7 +1130,7 @@ fn build_upvar_field_di_nodes<'ll, 'tcx>(
}
};

debug_assert!(
assert!(
up_var_tys.iter().all(|t| t == cx.tcx.normalize_erasing_regions(ParamEnv::reveal_all(), t))
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
let enum_type_and_layout = cx.layout_of(enum_type);
let enum_type_name = compute_debuginfo_type_name(cx.tcx, enum_type, false);

debug_assert!(!wants_c_like_enum_debuginfo(enum_type_and_layout));
assert!(!wants_c_like_enum_debuginfo(enum_type_and_layout));

type_map::build_type_with_children(
cx,
Expand Down Expand Up @@ -279,7 +279,7 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>(
let coroutine_type_and_layout = cx.layout_of(coroutine_type);
let coroutine_type_name = compute_debuginfo_type_name(cx.tcx, coroutine_type, false);

debug_assert!(!wants_c_like_enum_debuginfo(coroutine_type_and_layout));
assert!(!wants_c_like_enum_debuginfo(coroutine_type_and_layout));

type_map::build_type_with_children(
cx,
Expand Down Expand Up @@ -517,7 +517,7 @@ fn build_variant_struct_wrapper_type_di_node<'ll, 'tcx>(
if is_128_bits {
DiscrKind::Exact128(discr_val)
} else {
debug_assert_eq!(discr_val, discr_val as u64 as u128);
assert_eq!(discr_val, discr_val as u64 as u128);
DiscrKind::Exact(discr_val as u64)
}
}
Expand All @@ -526,8 +526,8 @@ fn build_variant_struct_wrapper_type_di_node<'ll, 'tcx>(
if is_128_bits {
DiscrKind::Range128(min, max)
} else {
debug_assert_eq!(min, min as u64 as u128);
debug_assert_eq!(max, max as u64 as u128);
assert_eq!(min, min as u64 as u128);
assert_eq!(max, max as u64 as u128);
DiscrKind::Range(min as u64, max as u64)
}
}
Expand Down Expand Up @@ -815,7 +815,7 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
}
}));

debug_assert_eq!(
assert_eq!(
cx.size_and_align_of(enum_type_and_layout.field(cx, tag_field).ty),
cx.size_and_align_of(super::tag_base_type(cx, enum_type_and_layout))
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ fn tag_base_type<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
enum_type_and_layout: TyAndLayout<'tcx>,
) -> Ty<'tcx> {
debug_assert!(match enum_type_and_layout.ty.kind() {
assert!(match enum_type_and_layout.ty.kind() {
ty::Coroutine(..) => true,
ty::Adt(adt_def, _) => adt_def.is_enum(),
_ => false,
Expand Down Expand Up @@ -251,7 +251,7 @@ fn build_enum_variant_struct_type_di_node<'ll, 'tcx>(
variant_layout: TyAndLayout<'tcx>,
di_flags: DIFlags,
) -> &'ll DIType {
debug_assert_eq!(variant_layout.ty, enum_type_and_layout.ty);
assert_eq!(variant_layout.ty, enum_type_and_layout.ty);

type_map::build_type_with_children(
cx,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(

let visibility_flags = visibility_di_flags(cx, enum_adt_def.did(), enum_adt_def.did());

debug_assert!(!wants_c_like_enum_debuginfo(enum_type_and_layout));
assert!(!wants_c_like_enum_debuginfo(enum_type_and_layout));

type_map::build_type_with_children(
cx,
Expand Down Expand Up @@ -142,7 +142,7 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>(
let containing_scope = get_namespace_for_item(cx, coroutine_def_id);
let coroutine_type_and_layout = cx.layout_of(coroutine_type);

debug_assert!(!wants_c_like_enum_debuginfo(coroutine_type_and_layout));
assert!(!wants_c_like_enum_debuginfo(coroutine_type_and_layout));

let coroutine_type_name = compute_debuginfo_type_name(cx.tcx, coroutine_type, false);

Expand Down
22 changes: 8 additions & 14 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata/type_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ mod private {

/// A unique identifier for anything that we create a debuginfo node for.
/// The types it contains are expected to already be normalized (which
/// is debug_asserted in the constructors).
/// is asserted in the constructors).
///
/// Note that there are some things that only show up in debuginfo, like
/// the separate type descriptions for each enum variant. These get an ID
Expand All @@ -58,12 +58,12 @@ pub(super) enum UniqueTypeId<'tcx> {

impl<'tcx> UniqueTypeId<'tcx> {
pub fn for_ty(tcx: TyCtxt<'tcx>, t: Ty<'tcx>) -> Self {
debug_assert_eq!(t, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), t));
assert_eq!(t, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), t));
UniqueTypeId::Ty(t, private::HiddenZst)
}

pub fn for_enum_variant_part(tcx: TyCtxt<'tcx>, enum_ty: Ty<'tcx>) -> Self {
debug_assert_eq!(enum_ty, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), enum_ty));
assert_eq!(enum_ty, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), enum_ty));
UniqueTypeId::VariantPart(enum_ty, private::HiddenZst)
}

Expand All @@ -72,7 +72,7 @@ impl<'tcx> UniqueTypeId<'tcx> {
enum_ty: Ty<'tcx>,
variant_idx: VariantIdx,
) -> Self {
debug_assert_eq!(enum_ty, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), enum_ty));
assert_eq!(enum_ty, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), enum_ty));
UniqueTypeId::VariantStructType(enum_ty, variant_idx, private::HiddenZst)
}

Expand All @@ -81,7 +81,7 @@ impl<'tcx> UniqueTypeId<'tcx> {
enum_ty: Ty<'tcx>,
variant_idx: VariantIdx,
) -> Self {
debug_assert_eq!(enum_ty, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), enum_ty));
assert_eq!(enum_ty, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), enum_ty));
UniqueTypeId::VariantStructTypeCppLikeWrapper(enum_ty, variant_idx, private::HiddenZst)
}

Expand All @@ -90,11 +90,8 @@ impl<'tcx> UniqueTypeId<'tcx> {
self_type: Ty<'tcx>,
implemented_trait: Option<PolyExistentialTraitRef<'tcx>>,
) -> Self {
debug_assert_eq!(
self_type,
tcx.normalize_erasing_regions(ParamEnv::reveal_all(), self_type)
);
debug_assert_eq!(
assert_eq!(self_type, tcx.normalize_erasing_regions(ParamEnv::reveal_all(), self_type));
assert_eq!(
implemented_trait,
tcx.normalize_erasing_regions(ParamEnv::reveal_all(), implemented_trait)
);
Expand Down Expand Up @@ -252,10 +249,7 @@ pub(super) fn build_type_with_children<'ll, 'tcx>(
members: impl FnOnce(&CodegenCx<'ll, 'tcx>, &'ll DIType) -> SmallVec<&'ll DIType>,
generics: impl FnOnce(&CodegenCx<'ll, 'tcx>) -> SmallVec<&'ll DIType>,
) -> DINodeCreationResult<'ll> {
debug_assert_eq!(
debug_context(cx).type_map.di_node_for_unique_id(stub_info.unique_type_id),
None
);
assert_eq!(debug_context(cx).type_map.di_node_for_unique_id(stub_info.unique_type_id), None);

debug_context(cx).type_map.insert(stub_info.unique_type_id, stub_info.metadata);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/debuginfo/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub(crate) fn fat_pointer_kind<'ll, 'tcx>(
ty::Dynamic(..) => Some(FatPtrKind::Dyn),
ty::Foreign(_) => {
// Assert that pointers to foreign types really are thin:
debug_assert_eq!(
assert_eq!(
cx.size_of(Ty::new_imm_ptr(cx.tcx, pointee_tail_ty)),
cx.size_of(Ty::new_imm_ptr(cx.tcx, cx.tcx.types.u8))
);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ fn exported_symbols_provider_local(
}
MonoItem::Fn(Instance { def: InstanceKind::DropGlue(def_id, Some(ty)), args }) => {
// A little sanity-check
debug_assert_eq!(
assert_eq!(
args.non_erasable_generics(tcx, def_id).next(),
Some(GenericArgKind::Type(ty))
);
Expand All @@ -370,7 +370,7 @@ fn exported_symbols_provider_local(
args,
}) => {
// A little sanity-check
debug_assert_eq!(
assert_eq!(
args.non_erasable_generics(tcx, def_id).next(),
Some(GenericArgKind::Type(ty))
);
Expand Down Expand Up @@ -462,7 +462,7 @@ fn upstream_monomorphizations_for_provider(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> Option<&UnordMap<GenericArgsRef<'_>, CrateNum>> {
debug_assert!(!def_id.is_local());
assert!(!def_id.is_local());
tcx.upstream_monomorphizations(()).get(&def_id)
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1512,7 +1512,7 @@ fn start_executing_work<B: ExtraBackendMethods>(
// We reduce the `running` counter by one. The
// `tokens.truncate()` below will take care of
// giving the Token back.
debug_assert!(running_with_own_token > 0);
assert!(running_with_own_token > 0);
running_with_own_token -= 1;
main_thread_state = MainThreadState::Lending;
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ fn push_debuginfo_type_name<'tcx>(
output: &mut String,
visited: &mut FxHashSet<Ty<'tcx>>,
) {
debug_assert!(!wants_c_like_enum_debuginfo(ty_and_layout));
assert!(!wants_c_like_enum_debuginfo(ty_and_layout));
output.push_str("enum2$<");
push_inner(output, visited);
push_close_angle_bracket(true, output);
Expand Down Expand Up @@ -660,7 +660,7 @@ fn push_generic_params_internal<'tcx>(
output: &mut String,
visited: &mut FxHashSet<Ty<'tcx>>,
) -> bool {
debug_assert_eq!(args, tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), args));
assert_eq!(args, tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), args));
let mut args = args.non_erasable_generics(tcx, def_id).peekable();
if args.peek().is_none() {
return false;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
}
if is_cleanupret {
// Cross-funclet jump - need a trampoline
debug_assert!(base::wants_new_eh_instructions(fx.cx.tcx().sess));
assert!(base::wants_new_eh_instructions(fx.cx.tcx().sess));
debug!("llbb_with_cleanup: creating cleanup trampoline for {:?}", target);
let name = &format!("{:?}_cleanup_trampoline_{:?}", self.bb, target);
let trampoline_llbb = Bx::append_block(fx.cx, fx.llfn, name);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ fn calculate_debuginfo_offset<
}
_ => {
// Sanity check for `can_use_in_debuginfo`.
debug_assert!(!elem.can_use_in_debuginfo());
assert!(!elem.can_use_in_debuginfo());
bug!("unsupported var debuginfo projection `{:?}`", projection)
}
}
Expand Down Expand Up @@ -502,7 +502,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

let DebugInfoOffset { direct_offset, indirect_offsets, result: fragment_layout } =
calculate_debuginfo_offset(bx, &fragment.projection, var_layout);
debug_assert!(indirect_offsets.is_empty());
assert!(indirect_offsets.is_empty());

if fragment_layout.size == Size::ZERO {
// Fragment is a ZST, so does not represent anything. Avoid generating anything
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
for elem in place_ref.projection.iter() {
match elem {
mir::ProjectionElem::Field(ref f, _) => {
debug_assert!(
assert!(
!o.layout.ty.is_any_ptr(),
"Bad PlaceRef: destructing pointers should use cast/PtrMetadata, \
but tried to access field {f:?} of pointer {o:?}",
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl<V: CodegenObject> PlaceValue<V> {

/// Creates a `PlaceRef` to this location with the given type.
pub fn with_type<'tcx>(self, layout: TyAndLayout<'tcx>) -> PlaceRef<'tcx, V> {
debug_assert!(
assert!(
layout.is_unsized() || layout.abi.is_uninhabited() || self.llextra.is_none(),
"Had pointer metadata {:?} for sized type {layout:?}",
self.llextra,
Expand Down Expand Up @@ -488,7 +488,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
cg_base = match *elem {
mir::ProjectionElem::Deref => bx.load_operand(cg_base).deref(bx.cx()),
mir::ProjectionElem::Field(ref field, _) => {
debug_assert!(
assert!(
!cg_base.layout.ty.is_any_ptr(),
"Bad PlaceRef: destructing pointers should use cast/PtrMetadata, \
but tried to access field {field:?} of pointer {cg_base:?}",
Expand Down
Loading

0 comments on commit ce7b069

Please sign in to comment.