Limit alias regions in Wasm-to-CLIF translation to 256

cranelift/codegen/src/context.rs

@@ -342,7 +342,7 @@ impl Context {
     /// by a store instruction to the same instruction (so-called
     /// "store-to-load forwarding").
     pub fn replace_redundant_loads(&mut self) -> CodegenResult<()> {
-        let mut analysis = AliasAnalysis::new(&self.func, &self.domtree);
+        let mut analysis = AliasAnalysis::new(&self.func, &self.cfg, &self.domtree);
         analysis.compute_and_update_aliases(&mut self.func, &self.cfg);
         Ok(())
     }
@@ -374,7 +374,7 @@ impl Context {
         );
         let fisa = fisa.into();
         self.compute_loop_analysis();
-        let mut alias_analysis = AliasAnalysis::new(&self.func, &self.domtree);
+        let mut alias_analysis = AliasAnalysis::new(&self.func, &self.cfg, &self.domtree);
         let mut pass = EgraphPass::new(
             &mut self.func,
             &self.domtree,

cranelift/codegen/src/egraph/mod.rs

@@ -1,7 +1,7 @@
 //! Support for egraphs represented in the DataFlowGraph.
 
 use crate::FxHashSet;
-use crate::alias_analysis::{AliasAnalysis, LastStores, OptResult};
+use crate::alias_analysis::{AliasAnalysis, MemoryState, OptResult};
 use crate::branch_to_trap::BranchToTrapAnalysis;
 use crate::ctxhash::{CtxEq, CtxHash, NullCtx};
 use crate::cursor::{Cursor, CursorPosition, FuncCursor};
@@ -155,7 +155,7 @@ where
     /// build a post-dominator tree for dead-store elimination.
     cfg: &'opt ControlFlowGraph,
     pub(crate) alias_analysis: &'opt mut AliasAnalysis<'analysis>,
-    pub(crate) alias_analysis_state: &'opt mut LastStores,
+    pub(crate) alias_analysis_state: &'opt mut MemoryState,
     pub(crate) branch_to_trap_analysis: &'opt mut BranchToTrapAnalysis,
     ctrl_plane: &'opt mut ControlPlane,
     // Held locally during optimization of one node (recursively):

cranelift/codegen/src/ir/memflags.rs

@@ -7,7 +7,7 @@ pub use crate::machinst::MachMemFlags;
 use alloc::borrow::Cow;
 use core::fmt;
 use core::hash::{Hash, Hasher};
-use core::ops::Index;
+use core::ops::{Index, IndexMut};
 use core::str::FromStr;
 use cranelift_entity::{entity_impl, packed_option::PackedOption};
 
@@ -38,20 +38,51 @@ entity_impl!(AliasRegion, "region");
 pub struct AliasRegionData {
     /// A unique, user-defined identifier for this alias region.
     ///
-    /// Alias regions are deduplicated based on this identifier.
-    ///
-    /// This deduplication happens during inlining, for example, when a
-    /// callee's alias regions are merged with the caller's. Therefore, when
-    /// inlining is enabled this identifier should be globally unique across
-    /// the whole compilation. When inlining is disabled, it is sufficient
-    /// to be unique within the context of a single function.
-    pub user_id: u32,
+    /// This must not change once the `AliasRegionData` is created, as
+    /// `AliasRegionSet` hash-conses based on this identifier.
+    user_id: u32,
 
     /// Description of this alias region, e.g. "vmctx", "funcref table",
     /// "global 42", or "gc struct `LinkedList` field `tail`".
     ///
     /// This only exists for printing in the CLIF text format.
-    pub description: Cow<'static, str>,
+    description: Cow<'static, str>,
+}
+
+impl AliasRegionData {
+    /// Create the data for an alias region with the given unique identifier
+    /// and human-readable description.
+    ///
+    /// Alias regions are deduplicated based on the `user_id`.
+    ///
+    /// This deduplication happens during inlining, for example, when a callee's
+    /// alias regions are merged with the caller's. Therefore, when inlining is
+    /// enabled this identifier should be globally unique across the whole
+    /// compilation. When inlining is disabled, it is sufficient to be unique
+    /// within the context of a single function.
+    ///
+    /// The `description` only exists for printing in the CLIF text format.
+    pub fn new(user_id: u32, description: impl Into<Cow<'static, str>>) -> Self {
+        Self {
+            user_id,
+            description: description.into(),
+        }
+    }
+
+    /// Get this region's unique identifier.
+    pub fn user_id(&self) -> u32 {
+        self.user_id
+    }
+
+    /// This region's human-readable description.
+    pub fn description(&self) -> &str {
+        &self.description
+    }
+
+    /// Get a mutable reference to this region's human-readable description.
+    pub fn description_mut(&mut self) -> &mut Cow<'static, str> {
+        &mut self.description
+    }
 }
 
 /// An opaque reference to memory operation flags stored in a
@@ -521,10 +552,10 @@ impl AliasRegionSet {
     /// Returns an existing `AliasRegion` if one with the same `user_id`
     /// already exists.
     pub fn insert(&mut self, data: AliasRegionData) -> AliasRegion {
-        if let Some(&existing) = self.dedupe_map.get(&data.user_id) {
+        if let Some(&existing) = self.dedupe_map.get(&data.user_id()) {
             return existing;
         }
-        let user_id = data.user_id;
+        let user_id = data.user_id();
         let key = self.alias_regions.push(data);
         self.dedupe_map.insert(user_id, key);
         key
@@ -535,7 +566,7 @@ impl AliasRegionSet {
     /// This is used by the CLIF text parser to faithfully represent the
     /// source text. The verifier will then check for duplicate `user_id`s.
     pub fn push(&mut self, data: AliasRegionData) -> AliasRegion {
-        let user_id = data.user_id;
+        let user_id = data.user_id();
         let key = self.alias_regions.push(data);
         self.dedupe_map.insert(user_id, key);
         key
@@ -574,8 +605,6 @@ impl AliasRegionSet {
     }
 }
 
-// NB: Do not implement `IndexMut` because alias region data is deduped and
-// shared by many mem flags.
 impl Index<AliasRegion> for AliasRegionSet {
     type Output = AliasRegionData;
 
@@ -584,6 +613,12 @@ impl Index<AliasRegion> for AliasRegionSet {
     }
 }
 
+impl IndexMut<AliasRegion> for AliasRegionSet {
+    fn index_mut(&mut self, ar: AliasRegion) -> &mut AliasRegionData {
+        &mut self.alias_regions[ar]
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

cranelift/codegen/src/verifier/mod.rs

@@ -420,16 +420,16 @@ impl<'a> Verifier<'a> {
     fn verify_alias_regions(&self, errors: &mut VerifierErrors) -> VerifierStepResult {
         let mut seen_user_ids = crate::HashMap::new();
         for (ar, ar_data) in self.func.dfg.alias_regions.iter() {
-            if let Some(&prev) = seen_user_ids.get(&ar_data.user_id) {
+            if let Some(&prev) = seen_user_ids.get(&ar_data.user_id()) {
                 errors.report((
                     ar,
                     format!(
                         "duplicate alias region user_id {}: {} and {}",
-                        ar_data.user_id, prev, ar
+                        ar_data.user_id(), prev, ar
                     ),
                 ));
             } else {
-                seen_user_ids.insert(ar_data.user_id, ar);
+                seen_user_ids.insert(ar_data.user_id(), ar);
             }
         }
         Ok(())

cranelift/codegen/src/write.rs

@@ -58,7 +58,7 @@ pub trait FuncWriter {
                 w,
                 func,
                 ar.into(),
-                &format_args!("{} \"{}\"", ar_data.user_id, ar_data.description),
+                &format_args!("{} \"{}\"", ar_data.user_id(), ar_data.description()),
             )?;
         }
 

cranelift/filetests/filetests/alias/fence-fallback-across-join.clif

@@ -0,0 +1,77 @@
+test optimize precise-output
+set opt_level=speed
+target aarch64
+
+function %fence_fallback_survives_join(i64, i32) -> i32, i32 {
+    region0 = 0 "R0"
+
+block0(v0: i64, v1: i32):
+    v3 = load.i32 notrap aligned region0 v0
+    brif v1, block1, block2
+
+block1:
+    jump block3
+
+block2:
+    jump block3
+
+block3:
+    v4 = load.i32 notrap aligned region0 v0
+    return v3, v4
+}
+
+; function %fence_fallback_survives_join(i64, i32) -> i32, i32 fast {
+;     region0 = 0 "R0"
+;
+; block0(v0: i64, v1: i32):
+;     v3 = load.i32 notrap aligned region0 v0
+;     brif v1, block1, block2
+;
+; block1:
+;     jump block3
+;
+; block2:
+;     jump block3
+;
+; block3:
+;     return v3, v3
+; }
+
+function %unknown_does_not_take_the_fence_fallback(i64, i32, i32) -> i32, i32 {
+    region0 = 0 "R0"
+
+block0(v0: i64, v1: i32, v2: i32):
+    v3 = load.i32 notrap aligned region0 v0
+    brif v1, block1, block2
+
+block1:
+    ;; This store prevents `block3` from reusing `v3`.
+    store notrap aligned region0 v2, v0
+    jump block3
+
+block2:
+    jump block3
+
+block3:
+    v4 = load.i32 notrap aligned region0 v0
+    return v3, v4
+}
+
+; function %unknown_does_not_take_the_fence_fallback(i64, i32, i32) -> i32, i32 fast {
+;     region0 = 0 "R0"
+;
+; block0(v0: i64, v1: i32, v2: i32):
+;     v3 = load.i32 notrap aligned region0 v0
+;     brif v1, block1, block2
+;
+; block1:
+;     store.i32 notrap aligned region0 v2, v0
+;     jump block3
+;
+; block2:
+;     jump block3
+;
+; block3:
+;     v4 = load.i32 notrap aligned region0 v0
+;     return v3, v4
+; }

cranelift/filetests/filetests/alias/join-order-independence.clif

@@ -0,0 +1,85 @@
+test optimize precise-output
+set opt_level=speed
+target aarch64
+
+;; The last-store analysis must not depend on the order in which the worklist
+;; happens to visit blocks. These two functions differ only in the order of the
+;; `brif`'s two arms, so they must optimize identically.
+
+function %arms_in_order(i64, i32, i32) -> i32, i32 {
+    region0 = 0 "R0"
+
+block0(v0: i64, v1: i32, v2: i32):
+    store notrap aligned region0 v1, v0
+    brif v1, block3, block1
+
+block1:
+    store notrap aligned region0 v2, v0
+    jump block3
+
+block3:
+    v3 = load.i32 notrap aligned region0 v0
+    jump block4
+
+block4:
+    v4 = load.i32 notrap aligned region0 v0
+    return v3, v4
+}
+
+; function %arms_in_order(i64, i32, i32) -> i32, i32 fast {
+;     region0 = 0 "R0"
+;
+; block0(v0: i64, v1: i32, v2: i32):
+;     store notrap aligned region0 v1, v0
+;     brif v1, block3, block1
+;
+; block1:
+;     store.i32 notrap aligned region0 v2, v0
+;     jump block3
+;
+; block3:
+;     v3 = load.i32 notrap aligned region0 v0
+;     jump block4
+;
+; block4:
+;     return v3, v3
+; }
+
+function %arms_swapped(i64, i32, i32) -> i32, i32 {
+    region0 = 0 "R0"
+
+block0(v0: i64, v1: i32, v2: i32):
+    store notrap aligned region0 v1, v0
+    brif v1, block1, block3
+
+block1:
+    store notrap aligned region0 v2, v0
+    jump block3
+
+block3:
+    v3 = load.i32 notrap aligned region0 v0
+    jump block4
+
+block4:
+    v4 = load.i32 notrap aligned region0 v0
+    return v3, v4
+}
+
+; function %arms_swapped(i64, i32, i32) -> i32, i32 fast {
+;     region0 = 0 "R0"
+;
+; block0(v0: i64, v1: i32, v2: i32):
+;     store notrap aligned region0 v1, v0
+;     brif v1, block1, block3
+;
+; block1:
+;     store.i32 notrap aligned region0 v2, v0
+;     jump block3
+;
+; block3:
+;     v3 = load.i32 notrap aligned region0 v0
+;     jump block4
+;
+; block4:
+;     return v3, v3
+; }