Repository: bytecodealliance/wasmtime
Pull request #5906 (2023-03-01 20:21:58)

x64: Improve codegen for i8x16.shr_u
This catches a case that wasn't handled previously by #5880 to allow a constant load to be folded into an instruction rather than forcing it to be loaded into a temporary register.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

- [ ] This has been discussed in issue #..., or if not, please tell us why
  here.
- [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.
- [ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the [code of
conduct](https://github.com/bytecodealliance/wasmtime/blob/master/CODE_OF_CONDUCT.md).
-->



Patch:
diff --git a/cranelift/codegen/meta/src/gen_inst.rs b/cranelift/codegen/meta/src/gen_inst.rs
index 01ccefb7e..488296999 100644
--- a/cranelift/codegen/meta/src/gen_inst.rs
+++ b/cranelift/codegen/meta/src/gen_inst.rs
@@ -66,7 +66,7 @@ fn gen_formats(formats: &[&InstructionFormat], fmt: &mut Formatter) {
 /// 16 bytes on 64-bit architectures. If more space is needed to represent an instruction, use a
 /// `ValueList` to store the additional information out of line.
 fn gen_instruction_data(formats: &[&InstructionFormat], fmt: &mut Formatter) {
-    fmt.line("#[derive(Copy, Clone, Debug, PartialEq, Hash)]");
+    fmt.line("#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]");
     fmt.line(r#"#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]"#);
     fmt.line("#[allow(missing_docs)]");
     fmtln!(fmt, "pub enum InstructionData {");
diff --git a/cranelift/codegen/src/egraph.rs b/cranelift/codegen/src/egraph.rs
index 2cbe412e3..e12b6fed3 100644
--- a/cranelift/codegen/src/egraph.rs
+++ b/cranelift/codegen/src/egraph.rs
@@ -7,13 +7,14 @@ use crate::dominator_tree::DominatorTree;
 use crate::egraph::domtree::DomTreeWithChildren;
 use crate::egraph::elaborate::Elaborator;
 use crate::fx::FxHashSet;
-use crate::inst_predicates::is_pure_for_egraph;
+use crate::inst_predicates::{is_mergeable_for_egraph, is_pure_for_egraph};
 use crate::ir::{
-    DataFlowGraph, Function, Inst, InstructionData, Type, Value, ValueDef, ValueListPool,
+    Block, DataFlowGraph, Function, Inst, InstructionData, Type, Value, ValueDef, ValueListPool,
 };
 use crate::loop_analysis::LoopAnalysis;
 use crate::opts::generated_code::ContextIter;
 use crate::opts::IsleContext;
+use crate::scoped_hash_map::{Entry as ScopedEntry, ScopedHashMap};
 use crate::trace;
 use crate::unionfind::UnionFind;
 use cranelift_entity::packed_option::ReservedValue;
@@ -77,6 +78,7 @@ where
     pub(crate) func: &'opt mut Function,
     pub(crate) value_to_opt_value: &'opt mut SecondaryMap<Value, Value>,
     pub(crate) gvn_map: &'opt mut CtxHashMap<(Type, InstructionData), Value>,
+    pub(crate) effectful_gvn_map: &'opt mut ScopedHashMap<(Type, InstructionData), Value>,
     pub(crate) eclasses: &'opt mut UnionFind<Value>,
     pub(crate) remat_values: &'opt mut FxHashSet<Value>,
     pub(crate) stats: &'opt mut Stats,
@@ -285,10 +287,49 @@ where
     fn optimize_skeleton_inst(&mut self, inst: Inst) -> bool {
         self.stats.skeleton_inst += 1;
 
-        // If a load or store, process it with the alias analysis to see
-        // if we can optimize it (rewrite in terms of an earlier load or
-        // stored value).
-        if let Some(new_result) =
+        // First, can we try to deduplicate? We need to keep some copy
+        // of the instruction around because it's side-effecting, but
+        // we may be able to reuse an earlier instance of it.
+        if is_mergeable_for_egraph(self.func, inst) {
+            let result = self.func.dfg.inst_results(inst)[0];
+            trace!(" -> mergeable side-effecting op {}", inst);
+
+            // Does this instruction already exist? If so, add entries to
+            // the value-map to rewrite uses of its results to the results
+            // of the original (existing) instruction. If not, optimize
+            // the new instruction.
+            //
+            // Note that we use the "effectful GVN map", which is
+            // scoped: because effectful ops are not removed from the
+            // skeleton (`Layout`), we need to be mindful of whether
+            // our current position is dominated by an instance of the
+            // instruction. (See #5796 for details.)
+            let ty = self.func.dfg.ctrl_typevar(inst);
+            match self
+                .effectful_gvn_map
+                .entry((ty, self.func.dfg.insts[inst].clone()))
+            {
+                ScopedEntry::Occupied(o) => {
+                    let orig_result = *o.get();
+                    // Hit in GVN map -- reuse value.
+                    self.value_to_opt_value[result] = orig_result;
+                    self.eclasses.union(orig_result, result);
+                    trace!(" -> merges result {} to {}", result, orig_result);
+                    true
+                }
+                ScopedEntry::Vacant(v) => {
+                    // Otherwise, insert it into the value-map.
+                    self.value_to_opt_value[result] = result;
+                    v.insert(result);
+                    trace!(" -> inserts as new (no GVN)");
+                    false
+                }
+            }
+        }
+        // Otherwise, if a load or store, process it with the alias
+        // analysis to see if we can optimize it (rewrite in terms of
+        // an earlier load or stored value).
+        else if let Some(new_result) =
             self.alias_analysis
                 .process_inst(self.func, self.alias_analysis_state, inst)
         {
@@ -382,83 +423,127 @@ impl<'a> EgraphPass<'a> {
         let mut cursor = FuncCursor::new(self.func);
         let mut value_to_opt_value: SecondaryMap<Value, Value> =
             SecondaryMap::with_default(Value::reserved_value());
+        // Map from instruction to value for hash-consing of pure ops
+        // into the egraph. This can be a standard (non-scoped)
+        // hashmap because pure ops have no location: they are
+        // "outside of" control flow.
+        //
+        // Note also that we keep the controlling typevar (the `Type`
+        // in the tuple below) because it may disambiguate
+        // instructions that are identical except for type.
         let mut gvn_map: CtxHashMap<(Type, InstructionData), Value> =
             CtxHashMap::with_capacity(cursor.func.dfg.num_values());
+        // Map from instruction to value for GVN'ing of effectful but
+        // idempotent ops, which remain in the side-effecting
+        // skeleton. This needs to be scoped because we cannot
+        // deduplicate one instruction to another that is in a
+        // non-dominating block.
+        //
+        // Note that we can use a ScopedHashMap here without the
+        // "context" (as needed by CtxHashMap) because in practice the
+        // ops we want to GVN have all their args inline. Equality on
+        // the InstructionData itself is conservative: two insts whose
+        // struct contents compare shallowly equal are definitely
+        // identical, but identical insts in a deep-equality sense may
+        // not compare shallowly equal, due to list indirection. This
+        // is fine for GVN, because it is still sound to skip any
+        // given GVN opportunity (and keep the original instructions).
+        //
+        // As above, we keep the controlling typevar here as part of
+        // the key: effectful instructions may (as for pure
+        // instructions) be differentiated only on the type.
+        let mut effectful_gvn_map: ScopedHashMap<(Type, InstructionData), Value> =
+            ScopedHashMap::new();
 
         // In domtree preorder, visit blocks. (TODO: factor out an
         // iterator from this and elaborator.)
         let root = self.domtree_children.root();
-        let mut block_stack = vec![root];
-        while let Some(block) = block_stack.pop() {
-            // We popped this block; push children
-            // immediately, then process this block.
-            block_stack.extend(self.domtree_children.children(block));
-
-            trace!("Processing block {}", block);
-            cursor.set_position(CursorPosition::Before(block));
-
-            let mut alias_analysis_state = self.alias_analysis.block_starting_state(block);
-
-            for &param in cursor.func.dfg.block_params(block) {
-                trace!("creating initial singleton eclass for blockparam {}", param);
-                self.eclasses.add(param);
-                value_to_opt_value[param] = param;
-            }
-            while let Some(inst) = cursor.next_inst() {
-                trace!("Processing inst {}", inst);
-
-                // While we're passing over all insts, create initial
-                // singleton eclasses for all result and blockparam
-                // values.  Also do initial analysis of all inst
-                // results.
-                for &result in cursor.func.dfg.inst_results(inst) {
-                    trace!("creating initial singleton eclass for {}", result);
-                    self.eclasses.add(result);
-                }
+        enum StackEntry {
+            Visit(Block),
+            Pop,
+        }
+        let mut block_stack = vec![StackEntry::Visit(root)];
+        while let Some(entry) = block_stack.pop() {
+            match entry {
+                StackEntry::Visit(block) => {
+                    // We popped this block; push children
+                    // immediately, then process this block.
+                    block_stack.push(StackEntry::Pop);
+                    block_stack
+                        .extend(self.domtree_children.children(block).map(StackEntry::Visit));
+                    effectful_gvn_map.increment_depth();
+
+                    trace!("Processing block {}", block);
+                    cursor.set_position(CursorPosition::Before(block));
+
+                    let mut alias_analysis_state = self.alias_analysis.block_starting_state(block);
+
+                    for &param in cursor.func.dfg.block_params(block) {
+                        trace!("creating initial singleton eclass for blockparam {}", param);
+                        self.eclasses.add(param);
+                        value_to_opt_value[param] = param;
+                    }
+                    while let Some(inst) = cursor.next_inst() {
+                        trace!("Processing inst {}", inst);
+
+                        // While we're passing over all insts, create initial
+                        // singleton eclasses for all result and blockparam
+                        // values.  Also do initial analysis of all inst
+                        // results.
+                        for &result in cursor.func.dfg.inst_results(inst) {
+                            trace!("creating initial singleton eclass for {}", result);
+                            self.eclasses.add(result);
+                        }
 
-                // Rewrite args of *all* instructions using the
-                // value-to-opt-value map.
-                cursor.func.dfg.resolve_aliases_in_arguments(inst);
-                cursor.func.dfg.map_inst_values(inst, |_, arg| {
-                    let new_value = value_to_opt_value[arg];
-                    trace!("rewriting arg {} of inst {} to {}", arg, inst, new_value);
-                    debug_assert_ne!(new_value, Value::reserved_value());
-                    new_value
-                });
-
-                // Build a context for optimization, with borrows of
-                // state. We can't invoke a method on `self` because
-                // we've borrowed `self.func` mutably (as
-                // `cursor.func`) so we pull apart the pieces instead
-                // here.
-                let mut ctx = OptimizeCtx {
-                    func: cursor.func,
-                    value_to_opt_value: &mut value_to_opt_value,
-                    gvn_map: &mut gvn_map,
-                    eclasses: &mut self.eclasses,
-                    rewrite_depth: 0,
-                    subsume_values: FxHashSet::default(),
-                    remat_values: &mut self.remat_values,
-                    stats: &mut self.stats,
-                    alias_analysis: self.alias_analysis,
-                    alias_analysis_state: &mut alias_analysis_state,
-                };
-
-                if is_pure_for_egraph(ctx.func, inst) {
-                    // Insert into GVN map and optimize any new nodes
-                    // inserted (recursively performing this work for
-                    // any nodes the optimization rules produce).
-                    let inst = NewOrExistingInst::Existing(inst);
-                    ctx.insert_pure_enode(inst);
-                    // We've now rewritten all uses, or will when we
-                    // see them, and the instruction exists as a pure
-                    // enode in the eclass, so we can remove it.
-                    cursor.remove_inst_and_step_back();
-                } else {
-                    if ctx.optimize_skeleton_inst(inst) {
-                        cursor.remove_inst_and_step_back();
+                        // Rewrite args of *all* instructions using the
+                        // value-to-opt-value map.
+                        cursor.func.dfg.resolve_aliases_in_arguments(inst);
+                        cursor.func.dfg.map_inst_values(inst, |_, arg| {
+                            let new_value = value_to_opt_value[arg];
+                            trace!("rewriting arg {} of inst {} to {}", arg, inst, new_value);
+                            debug_assert_ne!(new_value, Value::reserved_value());
+                            new_value
+                        });
+
+                        // Build a context for optimization, with borrows of
+                        // state. We can't invoke a method on `self` because
+                        // we've borrowed `self.func` mutably (as
+                        // `cursor.func`) so we pull apart the pieces instead
+                        // here.
+                        let mut ctx = OptimizeCtx {
+                            func: cursor.func,
+                            value_to_opt_value: &mut value_to_opt_value,
+                            gvn_map: &mut gvn_map,
+                            effectful_gvn_map: &mut effectful_gvn_map,
+                            eclasses: &mut self.eclasses,
+                            rewrite_depth: 0,
+                            subsume_values: FxHashSet::default(),
+                            remat_values: &mut self.remat_values,
+                            stats: &mut self.stats,
+                            alias_analysis: self.alias_analysis,
+                            alias_analysis_state: &mut alias_analysis_state,
+                        };
+
+                        if is_pure_for_egraph(ctx.func, inst) {
+                            // Insert into GVN map and optimize any new nodes
+                            // inserted (recursively performing this work for
+                            // any nodes the optimization rules produce).
+                            let inst = NewOrExistingInst::Existing(inst);
+                            ctx.insert_pure_enode(inst);
+                            // We've now rewritten all uses, or will when we
+                            // see them, and the instruction exists as a pure
+                            // enode in the eclass, so we can remove it.
+                            cursor.remove_inst_and_step_back();
+                        } else {
+                            if ctx.optimize_skeleton_inst(inst) {
+                                cursor.remove_inst_and_step_back();
+                            }
+                        }
                     }
                 }
+                StackEntry::Pop => {
+                    effectful_gvn_map.decrement_depth();
+                }
             }
         }
     }
diff --git a/cranelift/codegen/src/inst_predicates.rs b/cranelift/codegen/src/inst_predicates.rs
index 9b284e8b8..f82f41daa 100644
--- a/cranelift/codegen/src/inst_predicates.rs
+++ b/cranelift/codegen/src/inst_predicates.rs
@@ -73,6 +73,25 @@ pub fn is_pure_for_egraph(func: &Function, inst: Inst) -> bool {
     has_one_result && (is_readonly_load || (!op.can_load() && !trivially_has_side_effects(op)))
 }
 
+/// Can the given instruction be merged into another copy of itself?
+/// These instructions may have side-effects, but as long as we retain
+/// the first instance of the instruction, the second and further
+/// instances are redundant if they would produce the same trap or
+/// result.
+pub fn is_mergeable_for_egraph(func: &Function, inst: Inst) -> bool {
+    let op = func.dfg.insts[inst].opcode();
+    // We can only merge one-result operators due to the way that GVN
+    // is structured in the egraph implementation.
+    let has_one_result = func.dfg.inst_results(inst).len() == 1;
+    has_one_result
+        // Loads/stores are handled by alias analysis and not
+        // otherwise mergeable.
+        && !op.can_load()
+        && !op.can_store()
+        // Can only have idempotent side-effects.
+        && (!has_side_effect(func, inst) || op.side_effects_idempotent())
+}
+
 /// Does the given instruction have any side-effect as per [has_side_effect], or else is a load,
 /// but not the get_pinned_reg opcode?
 pub fn has_lowering_side_effect(func: &Function, inst: Inst) -> bool {
diff --git a/cranelift/codegen/src/ir/instructions.rs b/cranelift/codegen/src/ir/instructions.rs
index 97e0ac0b5..f28cc45d2 100644
--- a/cranelift/codegen/src/ir/instructions.rs
+++ b/cranelift/codegen/src/ir/instructions.rs
@@ -45,7 +45,7 @@ pub type ValueListPool = entity::ListPool<Value>;
 /// The BlockCall::new function guarantees this layout by requiring a block argument that's written
 /// in as the first element of the EntityList. Any subsequent entries are always assumed to be real
 /// Values.
-#[derive(Debug, Clone, Copy, PartialEq, Hash)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
 pub struct BlockCall {
     /// The underlying storage for the BlockCall. The first element of the values EntityList is
diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle
index 38c1f2de7..ec21968b3 100644
--- a/cranelift/codegen/src/isa/x64/lower.isle
+++ b/cranelift/codegen/src/isa/x64/lower.isle
@@ -522,12 +522,10 @@
             ;; Shift `src` using 16x8. Unfortunately, a 16x8 shift will only be
             ;; correct for half of the lanes; the others must be fixed up with
             ;; the mask below.
-            (unmasked Xmm (x64_psrlw src (mov_rmi_to_xmm masked_amt)))
-            (mask_addr SyntheticAmode (ushr_i8x16_mask masked_amt))
-            (mask Reg (x64_load $I8X16 mask_addr (ExtKind.None))))
+            (unmasked Xmm (x64_psrlw src (mov_rmi_to_xmm masked_amt))))
         (sse_and $I8X16
                  unmasked
-                 (RegMem.Reg mask))))
+                 (ushr_i8x16_mask masked_amt))))
 
 ;; Get the address of the mask to use when fixing up the lanes that weren't
 ;; correctly generated by the 16x8 shift.
@@ -3510,6 +3508,16 @@
 
 ;; Rules for `shuffle` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
+;; Special case for the `punpckhbw` instruction which interleaves the upper
+;; lanes of the two input registers.
+(rule 4 (lower (shuffle a b (u128_from_immediate 0x1f0f_1e0e_1d0d_1c0c_1b0b_1a0a_1909_1808)))
+      (x64_punpckhbw a b))
+
+;; Special case for the `punpcklbw` instruction which interleaves the lower
+;; lanes of the two input registers.
+(rule 4 (lower (shuffle a b (u128_from_immediate 0x1707_1606_1505_1404_1303_1202_1101_1000)))
+      (x64_punpcklbw a b))
+
 ;; If `lhs` and `rhs` are the same we can use a single PSHUFB to shuffle the XMM
 ;; register. We statically build `constructed_mask` to zero out any unknown lane
 ;; indices (may not be completely necessary: verification could fail incorrect
diff --git a/cranelift/entity/src/list.rs b/cranelift/entity/src/list.rs
index 659b94a9b..682eb256f 100644
--- a/cranelift/entity/src/list.rs
+++ b/cranelift/entity/src/list.rs
@@ -62,7 +62,7 @@ use serde::{Deserialize, Serialize};
 ///
 /// The index stored in an `EntityList` points to part 2, the list elements. The value 0 is
 /// reserved for the empty list which isn't allocated in the vector.
-#[derive(Clone, Copy, Debug, PartialEq, Hash)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
 pub struct EntityList<T: EntityRef + ReservedValue> {
     index: u32,


Test patch:
diff --git a/cranelift/filetests/filetests/egraph/issue-5796.clif b/cranelift/filetests/filetests/egraph/issue-5796.clif
new file mode 100644
index 000000000..66963f5c4
--- /dev/null
+++ b/cranelift/filetests/filetests/egraph/issue-5796.clif
@@ -0,0 +1,76 @@
+test optimize
+set opt_level=speed
+target x86_64
+
+function %f0(i32, i32, i32) -> i32 {
+block0(v0: i32, v1: i32, v2: i32):
+  brif v0, block1, block2
+
+block1:
+  v3 = udiv v1, v2
+  return v3
+
+block2:
+  v4 = udiv v1, v2
+  brif v1, block3, block4
+
+block3:
+  return v4
+
+block4:
+  return v4
+}
+
+; check: block0(v0: i32, v1: i32, v2: i32):
+; check:     brif v0, block1, block2
+
+; check: block1:
+; check:     v3 = udiv.i32 v1, v2
+; check:     return v3
+
+; check: block2:
+; check:     v4 = udiv.i32 v1, v2
+; check:     brif.i32 v1, block3, block4
+
+; check: block3:
+; check:     return v4
+
+; check: block4:
+; check:     return v4
+
+function %f1(i32, i32, i32) -> i32 {
+block0(v0: i32, v1: i32, v2: i32):
+  brif v0, block1, block2
+
+block1:
+  v3 = udiv v1, v2
+  return v3
+
+block2:
+  v4 = udiv v1, v2
+  brif v1, block3, block4
+
+block3:
+  v5 = udiv v1, v2
+  return v5
+
+block4:
+  return v4
+}
+
+; check: block0(v0: i32, v1: i32, v2: i32):
+; check:     brif v0, block1, block2
+
+; check: block1:
+; check:     v3 = udiv.i32 v1, v2
+; check:     return v3
+
+; check: block2:
+; check:     v4 = udiv.i32 v1, v2
+; check:     brif.i32 v1, block3, block4
+
+; check: block3:
+; check:     return v4
+
+; check: block4:
+; check:     return v4
diff --git a/cranelift/filetests/filetests/isa/aarch64/iconst-imm12_from_negated.isle b/cranelift/filetests/filetests/isa/aarch64/iconst-imm12_from_negated.isle
new file mode 100644
index 000000000..61067e316
--- /dev/null
+++ b/cranelift/filetests/filetests/isa/aarch64/iconst-imm12_from_negated.isle
@@ -0,0 +1,66 @@
+;; Test our lowerings that try to fit negated iconst values in immediates, like
+;; `x + -1 => sub x 0x1`. Currently, not working as expected for i32 and i16. 
+
+test compile precise-output
+set unwind_info=false
+target aarch64
+
+;; Result uses immediate as expected
+function %c(i64) -> i64 {
+block0(v0: i64):
+    v1 = iconst.i64 -1
+    v3 = iadd v0, v1
+    return v3
+}
+
+; VCode:
+; block0:
+;   sub x0, x0, #1
+;   ret
+; 
+; Disassembled:
+; block0: ; offset 0x0
+;   sub x0, x0, #1
+;   ret
+
+;; 4294967295 is zero-extended i32 -1
+;; Result should use immediate but currently doesn't
+function %b(i32) -> i32 {
+block0(v0: i32):
+    v1 = iconst.i32 4294967295 
+    v3 = iadd v0, v1
+    return v3
+}
+
+; VCode:
+; block0:
+;   movn w2, #0
+;   add w0, w0, w2
+;   ret
+; 
+; Disassembled:
+; block0: ; offset 0x0
+;   mov w2, #-1
+;   add w0, w0, w2
+;   ret
+
+;; 65535 is zero-extended i16 -1
+;; Result should use immediate but currently doesn't
+function %a(i16) -> i16 {
+block0(v0: i16):
+    v1 = iconst.i16 65535
+    v3 = iadd v0, v1
+    return v3
+}
+
+; VCode:
+; block0:
+;   movz w2, #65535
+;   add w0, w0, w2
+;   ret
+; 
+; Disassembled:
+; block0: ; offset 0x0
+;   mov w2, #0xffff
+;   add w0, w0, w2
+;   ret
diff --git a/cranelift/filetests/filetests/isa/x64/shuffle.clif b/cranelift/filetests/filetests/isa/x64/shuffle.clif
new file mode 100644
index 000000000..529b95cc5
--- /dev/null
+++ b/cranelift/filetests/filetests/isa/x64/shuffle.clif
@@ -0,0 +1,54 @@
+test compile precise-output
+set enable_simd
+target x86_64
+
+function %punpcklbw(i8x16, i8x16) -> i8x16 {
+block0(v0: i8x16, v1: i8x16):
+    v2 = shuffle v0, v1, 0x17071606150514041303120211011000
+    return v2
+}
+
+; VCode:
+;   pushq   %rbp
+;   movq    %rsp, %rbp
+; block0:
+;   punpcklbw %xmm0, %xmm1, %xmm0
+;   movq    %rbp, %rsp
+;   popq    %rbp
+;   ret
+; 
+; Disassembled:
+; block0: ; offset 0x0
+;   pushq %rbp
+;   movq %rsp, %rbp
+; block1: ; offset 0x4
+;   punpcklbw %xmm1, %xmm0
+;   movq %rbp, %rsp
+;   popq %rbp
+;   retq
+
+function %punpckhbw(i8x16, i8x16) -> i8x16 {
+block0(v0: i8x16, v1: i8x16):
+    v2 = shuffle v0, v1, 0x1f0f1e0e1d0d1c0c1b0b1a0a19091808
+    return v2
+}
+
+; VCode:
+;   pushq   %rbp
+;   movq    %rsp, %rbp
+; block0:
+;   punpckhbw %xmm0, %xmm1, %xmm0
+;   movq    %rbp, %rsp
+;   popq    %rbp
+;   ret
+; 
+; Disassembled:
+; block0: ; offset 0x0
+;   pushq %rbp
+;   movq %rsp, %rbp
+; block1: ; offset 0x4
+;   punpckhbw %xmm1, %xmm0
+;   movq %rbp, %rsp
+;   popq %rbp
+;   retq
+
diff --git a/cranelift/filetests/filetests/isa/x64/simd-arith-avx.clif b/cranelift/filetests/filetests/isa/x64/simd-arith-avx.clif
index 3e622bd5f..fdd58032e 100644
--- a/cranelift/filetests/filetests/isa/x64/simd-arith-avx.clif
+++ b/cranelift/filetests/filetests/isa/x64/simd-arith-avx.clif
@@ -1635,8 +1635,7 @@ block0(v0: i8x16, v1: i32):
 ;   vpsrlw  %xmm0, %xmm5, %xmm7
 ;   lea     const(0), %rsi
 ;   shlq    $4, %r10, %r10
-;   movdqu  0(%rsi,%r10,1), %xmm13
-;   vpand   %xmm7, %xmm13, %xmm0
+;   vpand   %xmm7, 0(%rsi,%r10,1), %xmm0
 ;   movq    %rbp, %rsp
 ;   popq    %rbp
 ;   ret
@@ -1652,11 +1651,13 @@ block0(v0: i8x16, v1: i32):
 ;   vpsrlw %xmm5, %xmm0, %xmm7
 ;   leaq 0x15(%rip), %rsi
 ;   shlq $4, %r10
-;   movdqu (%rsi, %r10), %xmm13
-;   vpand %xmm13, %xmm7, %xmm0
+;   vpand (%rsi, %r10), %xmm7, %xmm0
 ;   movq %rbp, %rsp
 ;   popq %rbp
 ;   retq
+;   addb %al, (%rax)
+;   addb %al, (%rax)
+;   addb %bh, %bh
 
 function %i8x16_ushr_imm(i8x16) -> i8x16 {
 block0(v0: i8x16):
@@ -1670,8 +1671,7 @@ block0(v0: i8x16):
 ;   movq    %rsp, %rbp
 ; block0:
 ;   vpsrlw  %xmm0, $1, %xmm2
-;   movdqu  const(0), %xmm4
-;   vpand   %xmm2, %xmm4, %xmm0
+;   vpand   %xmm2, const(0), %xmm0
 ;   movq    %rbp, %rsp
 ;   popq    %rbp
 ;   ret
@@ -1682,14 +1682,15 @@ block0(v0: i8x16):
 ;   movq %rsp, %rbp
 ; block1: ; offset 0x4
 ;   vpsrlw $1, %xmm0, %xmm2
-;   movdqu 0xf(%rip), %xmm4
-;   vpand %xmm4, %xmm2, %xmm0
+;   vpand 0xf(%rip), %xmm2, %xmm0
 ;   movq %rbp, %rsp
 ;   popq %rbp
 ;   retq
 ;   addb %al, (%rax)
 ;   addb %al, (%rax)
 ;   addb %al, (%rax)
+;   addb %al, (%rax)
+;   addb %al, (%rax)
 ;   jg 0xa1
 ;   jg 0xa3
 ;   jg 0xa5
diff --git a/cranelift/filetests/filetests/isa/x64/simd-bitwise-compile.clif b/cranelift/filetests/filetests/isa/x64/simd-bitwise-compile.clif
index 1ecdf31ed..f63cc2231 100644
--- a/cranelift/filetests/filetests/isa/x64/simd-bitwise-compile.clif
+++ b/cranelift/filetests/filetests/isa/x64/simd-bitwise-compile.clif
@@ -485,8 +485,7 @@ block0:
 ; block0:
 ;   movdqu  const(1), %xmm0
 ;   psrlw   %xmm0, $1, %xmm0
-;   movdqu  const(0), %xmm3
-;   pand    %xmm0, %xmm3, %xmm0
+;   pand    %xmm0, const(0), %xmm0
 ;   movq    %rbp, %rsp
 ;   popq    %rbp
 ;   ret
@@ -496,28 +495,21 @@ block0:
 ;   pushq %rbp
 ;   movq %rsp, %rbp
 ; block1: ; offset 0x4
-;   movdqu 0x34(%rip), %xmm0
+;   movdqu 0x24(%rip), %xmm0
 ;   psrlw $1, %xmm0
-;   movdqu 0x17(%rip), %xmm3
-;   pand %xmm3, %xmm0
+;   pand 7(%rip), %xmm0
 ;   movq %rbp, %rsp
 ;   popq %rbp
 ;   retq
 ;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   addb %al, (%rax)
-;   jg 0xb1
-;   jg 0xb3
-;   jg 0xb5
-;   jg 0xb7
-;   jg 0xb9
-;   jg 0xbb
-;   jg 0xbd
-;   jg 0xbf
+;   jg 0xa1
+;   jg 0xa3
+;   jg 0xa5
+;   jg 0xa7
+;   jg 0xa9
+;   jg 0xab
+;   jg 0xad
+;   jg 0xaf
 ;   addb %al, (%rcx)
 ;   addb (%rbx), %al
 ;   addb $5, %al
diff --git a/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory-egraph.wat b/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory-egraph.wat
new file mode 100644
index 000000000..b6d4e88dc
--- /dev/null
+++ b/cranelift/filetests/filetests/wasm/duplicate-loads-dynamic-memory-egraph.wat
@@ -0,0 +1,92 @@
+;;! target = "x86_64"
+;;!
+;;! optimize = true
+;;!
+;;! settings = [
+;;!   "enable_heap_access_spectre_mitigation=true",
+;;!   "opt_level=speed_and_size",
+;;!   "use_egraphs=true"
+;;! ]
+;;!
+;;! [globals.vmctx]
+;;! type = "i64"
+;;! vmctx = true
+;;!
+;;! [globals.heap_base]
+;;! type = "i64"
+;;! load = { base = "vmctx", offset = 0 }
+;;!
+;;! [globals.heap_bound]
+;;! type = "i64"
+;;! load = { base = "vmctx", offset = 8 }
+;;!
+;;! [[heaps]]
+;;! base = "heap_base"
+;;! min_size = 0
+;;! offset_guard_size = 0xffffffff
+;;! index_type = "i32"
+;;! style = { kind = "dynamic", bound = "heap_bound" }
+
+(module
+  (memory (export "memory") 0)
+  (func (export "load-without-offset") (param i32) (result i32 i32)
+    local.get 0
+    i32.load
+    local.get 0
+    i32.load
+  )
+  (func (export "load-with-offset") (param i32) (result i32 i32)
+    local.get 0
+    i32.load offset=1234
+    local.get 0
+    i32.load offset=1234
+  )
+)
+
+;; function u0:0(i32, i64 vmctx) -> i32, i32 fast {
+;;     gv0 = vmctx
+;;     gv1 = load.i64 notrap aligned gv0+8
+;;     gv2 = load.i64 notrap aligned gv0
+;;
+;;                                 block0(v0: i32, v1: i64):
+;; @0057                               v4 = uextend.i64 v0
+;; @0057                               v5 = iconst.i64 4
+;; @0057                               v6 = uadd_overflow_trap v4, v5, heap_oob  ; v5 = 4
+;; @0057                               v7 = load.i64 notrap aligned v1+8
+;; @0057                               v8 = load.i64 notrap aligned v1
+;; @0057                               v11 = icmp ugt v6, v7
+;; @0057                               v10 = iconst.i64 0
+;; @0057                               v9 = iadd v8, v4
+;; @0057                               v12 = select_spectre_guard v11, v10, v9  ; v10 = 0
+;; @0057                               v13 = load.i32 little heap v12
+;;                                     v2 -> v13
+;; @005f                               jump block1
+;;
+;;                                 block1:
+;; @005f                               return v13, v13
+;; }
+;;
+;; function u0:1(i32, i64 vmctx) -> i32, i32 fast {
+;;     gv0 = vmctx
+;;     gv1 = load.i64 notrap aligned gv0+8
+;;     gv2 = load.i64 notrap aligned gv0
+;;
+;;                                 block0(v0: i32, v1: i64):
+;; @0064                               v4 = uextend.i64 v0
+;; @0064                               v5 = iconst.i64 1238
+;; @0064                               v6 = uadd_overflow_trap v4, v5, heap_oob  ; v5 = 1238
+;; @0064                               v7 = load.i64 notrap aligned v1+8
+;; @0064                               v8 = load.i64 notrap aligned v1
+;; @0064                               v12 = icmp ugt v6, v7
+;; @0064                               v11 = iconst.i64 0
+;; @0064                               v9 = iadd v8, v4
+;;                                     v26 = iconst.i64 1234
+;; @0064                               v10 = iadd v9, v26  ; v26 = 1234
+;; @0064                               v13 = select_spectre_guard v12, v11, v10  ; v11 = 0
+;; @0064                               v14 = load.i32 little heap v13
+;;                                     v2 -> v14
+;; @006e                               jump block1
+;;
+;;                                 block1:
+;; @006e                               return v14, v14
+;; }