diff --git a/cranelift/codegen/src/egraph/elaborate.rs b/cranelift/codegen/src/egraph/elaborate.rs index 68f4238cfb..d52927ffea 100644 --- a/cranelift/codegen/src/egraph/elaborate.rs +++ b/cranelift/codegen/src/egraph/elaborate.rs @@ -278,6 +278,14 @@ impl<'a> Elaborator<'a> { before ); + // Get the best option; we use `value` (latest + // value) here so we have a full view of the + // eclass. + trace!("looking up best value for {}", value); + let (_, best_value) = self.value_to_best_value[value]; + debug_assert_ne!(best_value, Value::reserved_value()); + trace!("elaborate: value {} -> best {}", value, best_value); + let remat = if let Some(elab_val) = self.value_to_elaborated_value.get(&canonical_value) { @@ -287,7 +295,7 @@ impl<'a> Elaborator<'a> { // from another block. If so, ignore the hit // and recompute below. let remat = elab_val.in_block != self.cur_block - && self.remat_values.contains(&canonical_value); + && self.remat_values.contains(&best_value); if !remat { trace!("elaborate: value {} -> {:?}", value, elab_val); self.stats.elaborate_memoize_hit += 1; @@ -304,20 +312,12 @@ impl<'a> Elaborator<'a> { // Value not available; but still look up // whether it's been flagged for remat because // this affects placement. - let remat = self.remat_values.contains(&canonical_value); + let remat = self.remat_values.contains(&best_value); trace!(" -> not present in map; remat = {}", remat); remat }; self.stats.elaborate_memoize_miss += 1; - // Get the best option; we use `value` (latest - // value) here so we have a full view of the - // eclass. - trace!("looking up best value for {}", value); - let (_, best_value) = self.value_to_best_value[value]; - debug_assert_ne!(best_value, Value::reserved_value()); - trace!("elaborate: value {} -> best {}", value, best_value,); - // Now resolve the value to its definition to see // how we can compute it. let (inst, result_idx) = match self.func.dfg.value_def(best_value) { diff --git a/cranelift/filetests/filetests/egraph/issue-5716.clif b/cranelift/filetests/filetests/egraph/issue-5716.clif new file mode 100644 index 0000000000..7763d3eac8 --- /dev/null +++ b/cranelift/filetests/filetests/egraph/issue-5716.clif @@ -0,0 +1,41 @@ +;; This tests that a call does not get rematerialized, even if a remat flag is +;; set on a different node in its eclass. +;; +;; Below, `v97` is an add of `v238` (the call's first return value) and a +;; constant 0; a mid-end rule rewrites this to just `v238` (i.e., `v97` is unioned +;; in). Separately, a rule states that an add of a value and a constant always +;; gets rematerialized at use. When `v97` is used in a later block, it would have +;; rematerialized the add; except, if we instead use the result of the call +;; directly, we should *not* remat the call. If we do, a compile error results +;; later. + +test compile +set opt_level=speed_and_size +target aarch64 + +function u0:33() system_v { +ss0 = explicit_slot 32 +sig0 = (i64, i64, i64, i64, i64) -> i64, i64 system_v +fn0 = colocated u0:0 sig0 +jt0 = jump_table [block36, block38] +block0: + v80 = iconst.i32 0 + v91 = iconst.i64 0 + v92 = iconst.i64 0 + v96 = iconst.i64 0 + v235 = iconst.i64 0 + v236 = iconst.i64 0 + v237 = iconst.i64 0 + v238, v239 = call fn0(v236, v237, v91, v92, v235) ; v236 = 0, v237 = 0, v91 = 0, v92 = 0, v235 = 0 + v97 = iadd v238, v96 ; v96 = 0 + br_table v80, block37, jt0 ; v80 = 0 +block36: + trap user0 +block37: + trap unreachable +block38: + v98 = load.i8 notrap v97 + v99 = fcvt_from_uint.f64 v98 + stack_store v99, ss0 + trap user0 +}