Bug 1986850 - wasm-gc: Improve precision of alias-checking for structure loads/stores. r=bvisness,rhunt.

This improves alias checking for wasm wasm struct loads/stores, and has some
ridealong cleanups for other wasm alias-set markings.

* new method RefType::valuesMightAlias

* new method MWasmLoadField::mightAlias,
  with helper StructTypesMightBeRelatedByInheritance,
  which adds some better checks for struct aliasing

* ridealongs:
  - MWasmLoadField: set movable if there's no trap-site info
  - MWasmLoadField::congruentTo: tidy up (no functional change)

* ridealongs: MWasmNeg, MWasmStackArg, WasmRegisterResult,
  WasmFloatRegisterResult, WasmBuiltinFloatRegisterResult,
  WasmRegister64Result, MWasmRefAsNonNull, WasmRefCastAbstract,
  WasmRefCastConcrete: add missing empty-alias-set annotations,
  as detected during testing

* ridealong: MIROps.yaml: WasmBoundsCheckRange32: add missing `alias_set` of
  none and missing `guard` marking (the latter an outright bug)

Differential Revision: https://phabricator.services.mozilla.com/D264039

Author: julian-seward1

js/src/jit/MIR-wasm.h

@@ -579,6 +579,7 @@ class MWasmNeg : public MUnaryInstruction, public NoTypePolicy::Data {
  public:
   INSTRUCTION_HEADER(WasmNeg)
   TRIVIAL_NEW_WRAPPERS
+  AliasSet getAliasSet() const override { return AliasSet::None(); }
   ALLOW_CLONE(MWasmNeg)
 };
 
@@ -1635,6 +1636,10 @@ class MWasmParameter : public MNullaryInstruction {
  public:
   INSTRUCTION_HEADER(WasmParameter)
   TRIVIAL_NEW_WRAPPERS
+  // MWasmParameter has no getAliasSet routine.  Hence it acquires the default
+  // aliases-everything setting.  This doesn't matter in practice because these
+  // nodes only appear at the start of the function's entry block, and in any
+  // case they are not marked as movable.
 
   ABIArg abi() const { return abi_; }
 };
@@ -1681,6 +1686,9 @@ class MWasmStackArg : public MUnaryInstruction, public NoTypePolicy::Data {
 
   uint32_t spOffset() const { return spOffset_; }
   void incrementOffset(uint32_t inc) { spOffset_ += inc; }
+  AliasSet getAliasSet() const override {
+    return AliasSet::Store(AliasSet::Flag::Any);
+  }
 
   ALLOW_CLONE(MWasmStackArg)
 };
@@ -1710,6 +1718,7 @@ class MWasmRegisterResult : public MWasmResultBase<Register> {
  public:
   INSTRUCTION_HEADER(WasmRegisterResult)
   TRIVIAL_NEW_WRAPPERS
+  AliasSet getAliasSet() const override { return AliasSet::None(); }
 };
 
 class MWasmFloatRegisterResult : public MWasmResultBase<FloatRegister> {
@@ -1719,6 +1728,7 @@ class MWasmFloatRegisterResult : public MWasmResultBase<FloatRegister> {
  public:
   INSTRUCTION_HEADER(WasmFloatRegisterResult)
   TRIVIAL_NEW_WRAPPERS
+  AliasSet getAliasSet() const override { return AliasSet::None(); }
 };
 
 class MWasmBuiltinFloatRegisterResult : public MWasmResultBase<FloatRegister> {
@@ -1730,6 +1740,7 @@ class MWasmBuiltinFloatRegisterResult : public MWasmResultBase<FloatRegister> {
  public:
   INSTRUCTION_HEADER(WasmBuiltinFloatRegisterResult)
   TRIVIAL_NEW_WRAPPERS
+  AliasSet getAliasSet() const override { return AliasSet::None(); }
 
   bool hardFP() const { return hardFP_; }
 };
@@ -1741,6 +1752,7 @@ class MWasmRegister64Result : public MWasmResultBase<Register64> {
  public:
   INSTRUCTION_HEADER(WasmRegister64Result)
   TRIVIAL_NEW_WRAPPERS
+  AliasSet getAliasSet() const override { return AliasSet::None(); }
 };
 
 class MWasmStackResultArea : public MNullaryInstruction {
@@ -2559,7 +2571,10 @@ class MWasmLoadField : public MBinaryInstruction, public NoTypePolicy::Data {
         aliases.flags() == AliasSet::Load(AliasSet::Any).flags());
     setResultType(type);
     if (maybeTrap_) {
+      // This is safe, but see bug 1992059 for associated details.
       setGuard();
+    } else {
+      setMovable();
     }
     initWasmRefType(maybeRefType);
   }
@@ -2585,14 +2600,15 @@ class MWasmLoadField : public MBinaryInstruction, public NoTypePolicy::Data {
       return false;
     }
     const MWasmLoadField* other = ins->toWasmLoadField();
-    return ins->isWasmLoadField() && congruentIfOperandsEqual(ins) &&
-           offset() == other->offset() &&
+    return congruentIfOperandsEqual(other) && offset() == other->offset() &&
            structFieldIndex() == other->structFieldIndex() &&
            wideningOp() == other->wideningOp() &&
            getAliasSet().flags() == other->getAliasSet().flags() &&
            hierarchy() == other->hierarchy();
   }
 
+  virtual AliasType mightAlias(const MDefinition* ins) const override;
+
 #ifdef JS_JITSPEW
   void getExtras(ExtrasCollector* extras) const override {
     char buf[96];
@@ -2954,6 +2970,7 @@ class MWasmRefAsNonNull : public MUnaryInstruction, public NoTypePolicy::Data {
   }
 
   MDefinition* foldsTo(TempAllocator& alloc) override;
+  AliasSet getAliasSet() const override { return AliasSet::None(); }
 
   ALLOW_CLONE(MWasmRefAsNonNull)
 };
@@ -3061,6 +3078,7 @@ class MWasmRefCastAbstract : public MUnaryInstruction,
   INSTRUCTION_HEADER(WasmRefCastAbstract)
   TRIVIAL_NEW_WRAPPERS
   NAMED_OPERANDS((0, ref))
+  AliasSet getAliasSet() const override { return AliasSet::None(); }
 
   wasm::RefType destType() const { return destType_; };
   const wasm::TrapSiteDesc& trapSiteDesc() const { return trapSiteDesc_; }
@@ -3095,6 +3113,7 @@ class MWasmRefCastConcrete : public MBinaryInstruction,
   INSTRUCTION_HEADER(WasmRefCastConcrete)
   TRIVIAL_NEW_WRAPPERS
   NAMED_OPERANDS((0, ref), (1, superSTV))
+  AliasSet getAliasSet() const override { return AliasSet::None(); }
 
   wasm::RefType destType() const { return destType_; };
   const wasm::TrapSiteDesc& trapSiteDesc() const { return trapSiteDesc_; }

js/src/jit/MIR.cpp

@@ -8324,3 +8324,59 @@ bool MInt32ToStringWithBase::congruentTo(const MDefinition* ins) const {
   }
   return congruentIfOperandsEqual(ins);
 }
+
+// Returns `false` if it can be proven that (1) both `mtyA` and `mtyB` are
+// struct types and (2) they are not related by inheritance.  Returns `true` in
+// all other cases.  `true` is the safe-but-possibly-suboptimal return value.
+static bool StructTypesMightBeRelatedByInheritance(wasm::MaybeRefType mtyA,
+                                                   wasm::MaybeRefType mtyB) {
+  if (!mtyA.isSome() || !mtyB.isSome()) {
+    // The "Track Wasm ref types" pass couldn't establish that both `mtyA` and
+    // `mtyB` are ref types.  Give up.
+    return true;
+  }
+
+  wasm::RefType tyA = mtyA.value();
+  wasm::RefType tyB = mtyB.value();
+  if (!tyA.isTypeRef() || !tyA.typeDef()->isStructType() || !tyB.isTypeRef() ||
+      !tyB.typeDef()->isStructType()) {
+    // They aren't both struct types.  Give up.
+    return true;
+  }
+
+  // They are both struct types.  So they are related by inheritance if one is
+  // a subtype of the other.  (Which is also the case if they are the same
+  // type.)
+  return wasm::RefType::valuesMightAlias(tyA, tyB);
+}
+
+MDefinition::AliasType MWasmLoadField::mightAlias(
+    const MDefinition* ins) const {
+  if (!(getAliasSet().flags() & ins->getAliasSet().flags())) {
+    return AliasType::NoAlias;
+  }
+  MOZ_ASSERT(!isEffectful() && ins->isEffectful());
+
+  // Pick off cases where we can easily prove non-aliasing.  The idea is that
+  // two struct field accesses can't alias if either they are at different
+  // offsets, or the struct types are unrelated (which implies that the struct
+  // base pointer for one of the accesses could not validly be handed to the
+  // other access).
+  if (ins->isWasmStoreField()) {
+    const MWasmStoreField* store = ins->toWasmStoreField();
+    if (offset() != store->offset() ||
+        !StructTypesMightBeRelatedByInheritance(base()->wasmRefType(),
+                                                store->base()->wasmRefType())) {
+      return AliasType::NoAlias;
+    }
+  } else if (ins->isWasmStoreFieldRef()) {
+    const MWasmStoreFieldRef* store = ins->toWasmStoreFieldRef();
+    if (offset() != store->offset() ||
+        !StructTypesMightBeRelatedByInheritance(base()->wasmRefType(),
+                                                store->base()->wasmRefType())) {
+      return AliasType::NoAlias;
+    }
+  }
+
+  return AliasType::MayAlias;
+}

js/src/jit/MIROps.yaml

@@ -3957,6 +3957,8 @@
   type_policy: none
   generate_lir: true
   lir_temps: 1
+  alias_set: none
+  guard: true
 
 - name: WasmExtendU32Index
   operands:

js/src/wasm/WasmValType.h

@@ -444,6 +444,17 @@ class RefType {
   static bool isSubTypeOf(RefType subType, RefType superType);
   static bool castPossible(RefType sourceType, RefType destType);
 
+  // If we have two references, one of type `a` and one of type `b`, return
+  // true if there is any possibility that they might point at the same thing.
+  // That can only happen if either they are the same type or if one type is a
+  // subtype of the other.  Note, this can only be used for types in the same
+  // hierarchy.
+  static bool valuesMightAlias(RefType a, RefType b) {
+    MOZ_RELEASE_ASSERT(a.hierarchy() == b.hierarchy());
+    // The exact-same-type case is subsumed by `isSubTypeOf`.
+    return RefType::isSubTypeOf(a, b) || RefType::isSubTypeOf(b, a);
+  }
+
   // Gets the top of the given type's hierarchy, e.g. Any for structs and
   // arrays, and Func for funcs.
   RefType topType() const;