diff --git a/majit/majit-metainterp/src/optimizeopt/heap.rs b/majit/majit-metainterp/src/optimizeopt/heap.rs index b279f98a984..dd1641d1c8c 100644 --- a/majit/majit-metainterp/src/optimizeopt/heap.rs +++ b/majit/majit-metainterp/src/optimizeopt/heap.rs @@ -861,26 +861,6 @@ impl OptHeap { } } - /// Is a `w_class` store still pending under ANY descr spelling? - /// - /// The field caches are keyed by `Arc::as_ptr` identity - /// (`cached_field_pos_for_descr`), and pyre mints more than one descr for - /// the `w_class` header (a walker singleton with no parent, and the size - /// descr's own `all_fielddescrs` entry). A miss on the descr being read - /// therefore does not prove the field is untouched — the store may sit in - /// the other spelling's `lazy_set`. Asking across every spelling costs a - /// fold, never correctness. - /// - /// Only `lazy_set` is consulted: a store that has already been flushed - /// (`force_lazy_set` → `put_field_back_to_info`) is readable from the - /// instance's own field slot, and `cached_structs` also fills up from plain - /// reads, which say nothing about the field having been written. - fn w_class_set_pending(&self) -> bool { - self.cached_fields.iter().any(|(_, descr, cf)| { - descr.as_field_descr().is_some_and(|fd| fd.is_w_class()) && cf.lazy_set.is_some() - }) - } - fn cached_field_pos_for_descr(&self, descr: &DescrRef) -> Option { let identity = descr_identity(descr); self.cached_fields @@ -2227,75 +2207,6 @@ impl OptHeap { // Cache miss — fall through to emit the getfield } - // Pyre object-model: `w_class` (PyObject header) carries Python-level - // class identity. `virtualize.rs` resolves it from a virtual's own - // class; once the allocation is forced the read arrives here instead. - // `info.py:152 force_box` keeps the class (`info.py:324 - // get_known_class` still answers), so a forced instance still names its - // layout and the header value is that layout's canonical class. - // - // A store that reached the instance's own slot wins over the layout's - // canonical class; only when the header was never written does the - // layout answer, and then only if no store is still pending. A retag to - // a user subclass writes this header, and folding past that write would - // answer with the base class and silently defeat a class check. Reads - // of an object whose layout is unknown (`descr: None`, what - // `ensure_ptr_info_arg0` builds for a plain heap object) resolve to - // nothing and stay in the trace. - // - // The slot is looked up through the LAYOUT's own `w_class` fielddescr, - // never the read descr's index: a parent-bearing `w_class` spelling has - // `index_in_parent == 0`, which is the first VALUE field, and folding - // through it forwards `Ref <- Int` (`virtualize.rs:850-856`). - if descr.as_field_descr().is_some_and(|fd| fd.is_w_class()) { - let obj_box = ctx.get_box_replacement_operand(obj); - if let Some(crate::optimizeopt::info::PtrInfo::Instance(iinfo)) = - ctx.peek_ptr_info(&obj_box) - { - let size_descr = iinfo.descr.as_ref().and_then(|d| d.as_size_descr()); - let stored = size_descr - .and_then(|sd| { - sd.all_fielddescrs() - .iter() - .find(|fd| fd.is_w_class()) - .map(|fd| fd.index_in_parent() as u32) - }) - .and_then(|widx| { - iinfo - .fields - .iter() - .find(|(idx, _)| *idx == widx) - .map(|(_, e)| e.clone()) - }); - match stored { - Some(crate::optimizeopt::info::FieldEntry::Value(b_val)) => { - let b_old = Operand::from_bound_op(op_rc); - let b_val = ctx.get_box_replacement_operand(b_val.to_opref()); - ctx.make_equal_to(&b_old, &b_val); - return OptimizationResult::Remove; - } - // Not a value yet — leave the read rather than folding past - // the pending preamble op. - Some(crate::optimizeopt::info::FieldEntry::Preamble(_)) => {} - None => { - if !self.w_class_set_pending() { - if let Some(w_class) = size_descr - .and_then(|sd| sd.w_class_obj()) - .filter(|&w| w != 0) - { - let b = ctx.materialize_operand_at(op.pos.get()); - ctx.make_constant_box( - &b, - majit_ir::Value::Ref(majit_ir::GcRef(w_class as usize)), - ); - return OptimizationResult::Remove; - } - } - } - } - } - } - // Virtualizable fields are loop-variant; skip caching/import. let is_vable_field = descr.is_virtualizable(); diff --git a/majit/majit-metainterp/src/optimizeopt/unroll.rs b/majit/majit-metainterp/src/optimizeopt/unroll.rs index 01223d8ba8b..8cc3d1831a2 100644 --- a/majit/majit-metainterp/src/optimizeopt/unroll.rs +++ b/majit/majit-metainterp/src/optimizeopt/unroll.rs @@ -4040,10 +4040,7 @@ impl OptUnroll { // producer first, since the top-of-loop _map_args above would // otherwise read an unmapped arg. let grown = current_short_jump_args(short_preamble, ctx); - // `<=`, not `==`: the list only grows here, but the slice below - // indexes from `num_short_jump_args`, so a shrink would panic - // rather than fall into the "did not grow" exit. - if grown.len() <= num_short_jump_args { + if grown.len() == num_short_jump_args { break; } let all_new_mapped = grown[num_short_jump_args..] diff --git a/majit/majit-metainterp/src/optimizeopt/virtualize.rs b/majit/majit-metainterp/src/optimizeopt/virtualize.rs index 13ffac1b4a8..39e940a7f97 100644 --- a/majit/majit-metainterp/src/optimizeopt/virtualize.rs +++ b/majit/majit-metainterp/src/optimizeopt/virtualize.rs @@ -893,18 +893,27 @@ impl OptVirtualize { // rather than mis-indexing a value field. return OptimizationResult::PassOn; } - // Once the allocation is forced the read is NOT resolved here. - // `virtualize.py:184-195 optimize_GETFIELD_GC_*` folds only - // under `opinfo.is_virtual()` and otherwise emits, leaving a - // non-virtual's field read to `heap.py`. That layering is - // load-bearing for this field: `force_box` empties the forced - // instance's field list (info.rs:1113-1118, so heap's - // `do_setfield` cannot MUST_ALIAS-elide the materializing - // SETFIELD_GC) and routes the write into `OptHeap`, where it - // sits in `CachedField::lazy_set`. Folding here would run - // BEFORE that pass and answer from the layout's canonical - // class, discarding a pending retag to a user subclass. - // The canonical-class fallback lives in `optimize_getfield`. + // Once the allocation is forced the read is NOT resolved — not + // here and not in `optimize_getfield`. `virtualize.py:184-195 + // optimize_GETFIELD_GC_*` folds only under + // `opinfo.is_virtual()` and otherwise emits; upstream has no + // counterpart to resolve afterwards because + // `jtransform.py:1004-1009 handle_getfield_typeptr` deletes the + // read at codewriter time, so no typeptr getfield ever reaches + // the optimizer. + // + // Answering from the layout's canonical class here is unsound: + // `force_box` empties the forced instance's field list + // (info.rs:1113-1118, so heap's `do_setfield` cannot + // MUST_ALIAS-elide the materializing SETFIELD_GC) and routes + // the header write into `OptHeap`, where it sits in + // `CachedField::lazy_set`. A retag to a user subclass would be + // discarded and the base class answered instead. Resolving it + // in `OptHeap` does not close the hole either: reads and writes + // of this header carry different descr spellings, and the + // caches are keyed by `Arc::as_ptr` while `structinfo_setfield` + // slots by `field_slot_index`, so the two do not meet. Removing + // that split is the prerequisite for folding this read at all. } let field_val = match &info { PtrInfo::Virtual(vinfo) => get_field(&vinfo.fields, field_idx),