[GC] Fix Array optimization issues (#3438)

Precompute still tried to precompute a reference because the check was
not in the topmost place.

Also we truncated i8/i16 values, but did not extend them properly. That
was also an issue with structs.

The new test replaces the old one by moving from -O1 to -Oz (which
runs more opts, and would have noticed this earlier), and adds array
operations too, including sign extends.

1 files changed, 32 insertions, 8 deletions

diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
index d701aaf15..e4c20082e 100644
--- a/src/wasm-interpreter.h
+++ b/src/wasm-interpreter.h
@@ -1427,7 +1427,8 @@ public:
     if (!data) {
       trap("null ref");
     }
-    return (*data)[curr->index];
+    auto field = curr->ref->type.getHeapType().getStruct().fields[curr->index];
+    return extendForPacking((*data)[curr->index], field, curr->signed_);
   }
   Flow visitStructSet(StructSet* curr) {
     NOTE_ENTER("StructSet");
@@ -1444,7 +1445,7 @@ public:
       trap("null ref");
     }
     auto field = curr->ref->type.getHeapType().getStruct().fields[curr->index];
-    (*data)[curr->index] = getMaybePackedValue(value.getSingleValue(), field);
+    (*data)[curr->index] = truncateForPacking(value.getSingleValue(), field);
     return Flow();
   }
   Flow visitArrayNew(ArrayNew* curr) {
@@ -1494,7 +1495,8 @@ public:
     if (i >= data->size()) {
       trap("array oob");
     }
-    return (*data)[i];
+    auto field = curr->ref->type.getHeapType().getArray().element;
+    return extendForPacking((*data)[i], field, curr->signed_);
   }
   Flow visitArraySet(ArraySet* curr) {
     NOTE_ENTER("ArraySet");
@@ -1519,7 +1521,7 @@ public:
       trap("array oob");
     }
     auto field = curr->ref->type.getHeapType().getArray().element;
-    (*data)[i] = getMaybePackedValue(value.getSingleValue(), field);
+    (*data)[i] = truncateForPacking(value.getSingleValue(), field);
     return Flow();
   }
   Flow visitArrayLen(ArrayLen* curr) {
@@ -1541,13 +1543,35 @@ public:
 
 private:
   // Truncate the value if we need to. The storage is just a list of Literals,
-  // so we can't just write the value like we would to a C struct field.
-  Literal getMaybePackedValue(Literal value, const Field& field) {
+  // so we can't just write the value like we would to a C struct field and
+  // expect it to truncate for us. Instead, we truncate so the stored value is
+  // proper for the type.
+  Literal truncateForPacking(Literal value, const Field& field) {
+    if (field.type == Type::i32) {
+      int32_t c = value.geti32();
+      if (field.packedType == Field::i8) {
+        value = Literal(c & 0xff);
+      } else if (field.packedType == Field::i16) {
+        value = Literal(c & 0xffff);
+      }
+    }
+    return value;
+  }
+
+  Literal extendForPacking(Literal value, const Field& field, bool signed_) {
     if (field.type == Type::i32) {
+      int32_t c = value.geti32();
       if (field.packedType == Field::i8) {
-        value = value.and_(Literal(int32_t(0xff)));
+        // The stored value should already be truncated.
+        assert(c == (c & 0xff));
+        if (signed_) {
+          value = Literal((c << 24) >> 24);
+        }
       } else if (field.packedType == Field::i16) {
-        value = value.and_(Literal(int32_t(0xffff)));
+        assert(c == (c & 0xffff));
+        if (signed_) {
+          value = Literal((c << 16) >> 16);
+        }
       }
     }
     return value;