Several refactorings to constant.rs

* Move the done hash set from ConstantCx to define_all_allocs.
* Move check if alloc has already been defined to the start of the loop.
* Extract helper function for vtables.

Author: bjorn3

src/constant.rs

@@ -6,30 +6,28 @@ use cranelift_module::*;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::mir::interpret::{read_target_uint, AllocId, GlobalAlloc, Scalar};
-use rustc_middle::ty::ScalarInt;
+use rustc_middle::ty::{Binder, ExistentialTraitRef, ScalarInt};
 
 use crate::prelude::*;
 
 pub(crate) struct ConstantCx {
     todo: Vec<TodoItem>,
-    done: FxHashSet<DataId>,
     anon_allocs: FxHashMap<AllocId, DataId>,
 }
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
 enum TodoItem {
     Alloc(AllocId),
     Static(DefId),
 }
 
 impl ConstantCx {
     pub(crate) fn new() -> Self {
-        ConstantCx { todo: vec![], done: FxHashSet::default(), anon_allocs: FxHashMap::default() }
+        ConstantCx { todo: vec![], anon_allocs: FxHashMap::default() }
     }
 
     pub(crate) fn finalize(mut self, tcx: TyCtxt<'_>, module: &mut dyn Module) {
         define_all_allocs(tcx, module, &mut self);
-        self.done.clear();
     }
 }
 
@@ -153,14 +151,12 @@ pub(crate) fn codegen_const_value<'tcx>(
                         fx.bcx.ins().func_addr(fx.pointer_type, local_func_id)
                     }
                     GlobalAlloc::VTable(ty, trait_ref) => {
-                        let alloc_id = fx.tcx.vtable_allocation((ty, trait_ref));
-                        let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory();
-                        // FIXME: factor this common code with the `Memory` arm into a function?
-                        let data_id = data_id_for_alloc_id(
+                        let data_id = data_id_for_vtable(
+                            fx.tcx,
                             &mut fx.constants_cx,
                             fx.module,
-                            alloc_id,
-                            alloc.inner().mutability,
+                            ty,
+                            trait_ref,
                         );
                         let local_data_id =
                             fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
@@ -208,12 +204,8 @@ fn pointer_for_allocation<'tcx>(
     alloc_id: AllocId,
 ) -> crate::pointer::Pointer {
     let alloc = fx.tcx.global_alloc(alloc_id).unwrap_memory();
-    let data_id = data_id_for_alloc_id(
-        &mut fx.constants_cx,
-        &mut *fx.module,
-        alloc_id,
-        alloc.inner().mutability,
-    );
+    let data_id =
+        data_id_for_alloc_id(&mut fx.constants_cx, fx.module, alloc_id, alloc.inner().mutability);
 
     let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
     if fx.clif_comments.enabled() {
@@ -235,6 +227,17 @@ pub(crate) fn data_id_for_alloc_id(
         .or_insert_with(|| module.declare_anonymous_data(mutability.is_mut(), false).unwrap())
 }
 
+pub(crate) fn data_id_for_vtable<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    cx: &mut ConstantCx,
+    module: &mut dyn Module,
+    ty: Ty<'tcx>,
+    trait_ref: Option<Binder<'tcx, ExistentialTraitRef<'tcx>>>,
+) -> DataId {
+    let alloc_id = tcx.vtable_allocation((ty, trait_ref));
+    data_id_for_alloc_id(cx, module, alloc_id, Mutability::Not)
+}
+
 fn data_id_for_static(
     tcx: TyCtxt<'_>,
     module: &mut dyn Module,
@@ -327,7 +330,12 @@ fn data_id_for_static(
 }
 
 fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut ConstantCx) {
+    let mut done = FxHashSet::default();
     while let Some(todo_item) = cx.todo.pop() {
+        if !done.insert(todo_item) {
+            continue;
+        }
+
         let (data_id, alloc, section_name) = match todo_item {
             TodoItem::Alloc(alloc_id) => {
                 let alloc = match tcx.global_alloc(alloc_id) {
@@ -358,10 +366,6 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
             }
         };
 
-        if cx.done.contains(&data_id) {
-            continue;
-        }
-
         let mut data = DataDescription::new();
         let alloc = alloc.inner();
         data.set_align(alloc.align.bytes());
@@ -418,8 +422,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
                     data_id_for_alloc_id(cx, module, alloc_id, target_alloc.inner().mutability)
                 }
                 GlobalAlloc::VTable(ty, trait_ref) => {
-                    let alloc_id = tcx.vtable_allocation((ty, trait_ref));
-                    data_id_for_alloc_id(cx, module, alloc_id, Mutability::Not)
+                    data_id_for_vtable(tcx, cx, module, ty, trait_ref)
                 }
                 GlobalAlloc::Static(def_id) => {
                     if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::THREAD_LOCAL)
@@ -446,7 +449,6 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
         }
 
         module.define_data(data_id, &data).unwrap();
-        cx.done.insert(data_id);
     }
 
     assert!(cx.todo.is_empty(), "{:?}", cx.todo);

src/vtable.rs

@@ -2,7 +2,7 @@
 //!
 //! See `rustc_codegen_ssa/src/meth.rs` for reference.
 
-use crate::constant::data_id_for_alloc_id;
+use crate::constant::data_id_for_vtable;
 use crate::prelude::*;
 
 pub(crate) fn vtable_memflags() -> MemFlags {
@@ -92,12 +92,10 @@ pub(crate) fn get_vtable<'tcx>(
     ty: Ty<'tcx>,
     trait_ref: Option<ty::PolyExistentialTraitRef<'tcx>>,
 ) -> Value {
-    let alloc_id = fx.tcx.vtable_allocation((ty, trait_ref));
-    let data_id =
-        data_id_for_alloc_id(&mut fx.constants_cx, &mut *fx.module, alloc_id, Mutability::Not);
+    let data_id = data_id_for_vtable(fx.tcx, &mut fx.constants_cx, fx.module, ty, trait_ref);
     let local_data_id = fx.module.declare_data_in_func(data_id, fx.bcx.func);
     if fx.clif_comments.enabled() {
-        fx.add_comment(local_data_id, format!("vtable: {:?}", alloc_id));
+        fx.add_comment(local_data_id, "vtable");
     }
     fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
 }