Heap2Local: Fix signed struct/array reads (#6484)

In #6480 I forgot that StructGet can be signed, which means we need to emit
a sign-extend.

Arrays already copied the field as part of Array2Struct.

2 files changed, 68 insertions, 16 deletions

diff --git a/src/passes/Heap2Local.cpp b/src/passes/Heap2Local.cpp
index 3b0410a77..b77775462 100644
--- a/src/passes/Heap2Local.cpp
+++ b/src/passes/Heap2Local.cpp
@@ -652,7 +652,7 @@ struct Struct2Local : PostWalker<Struct2Local> {
   // Add a mask for packed fields. We add masks on sets rather than on gets
   // because gets tend to be more numerous both in code appearances and in
   // runtime execution. As a result of masking on sets, the value in the local
-  // is always the masked value (which is also nice for debugging,
+  // is always the masked unsigned value (which is also nice for debugging,
   // incidentally).
   Expression* addMask(Expression* value, const Field& field) {
     if (!field.isPacked()) {
@@ -795,7 +795,8 @@ struct Struct2Local : PostWalker<Struct2Local> {
       return;
     }
 
-    auto type = fields[curr->index].type;
+    auto& field = fields[curr->index];
+    auto type = field.type;
     if (type != curr->type) {
       // Normally we are just replacing a struct.get with a local.get of a
       // local that was created to have the same type as the struct's field,
@@ -811,9 +812,13 @@ struct Struct2Local : PostWalker<Struct2Local> {
       // which may be more refined.
       refinalize = true;
     }
-    replaceCurrent(builder.makeSequence(
-      builder.makeDrop(curr->ref),
-      builder.makeLocalGet(localIndexes[curr->index], type)));
+    Expression* value = builder.makeLocalGet(localIndexes[curr->index], type);
+    if (field.isPacked() && curr->signed_) {
+      // The value in the local is the masked unsigned value, which we must
+      // sign-extend.
+      value = Bits::makeSignExt(value, field.getByteSize(), wasm);
+    }
+    replaceCurrent(builder.makeSequence(builder.makeDrop(curr->ref), value));
   }
 };
 
diff --git a/test/lit/passes/heap2local.wast b/test/lit/passes/heap2local.wast
index 96fef4701..197b86905 100644
--- a/test/lit/passes/heap2local.wast
+++ b/test/lit/passes/heap2local.wast
@@ -221,6 +221,20 @@
   ;; CHECK-NEXT:    (local.get $1)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (block (result i32)
+  ;; CHECK-NEXT:    (drop
+  ;; CHECK-NEXT:     (ref.null none)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (i32.shr_s
+  ;; CHECK-NEXT:     (i32.shl
+  ;; CHECK-NEXT:      (local.get $1)
+  ;; CHECK-NEXT:      (i32.const 24)
+  ;; CHECK-NEXT:     )
+  ;; CHECK-NEXT:     (i32.const 24)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
   ;; CHECK-NEXT:  (block
   ;; CHECK-NEXT:   (drop
   ;; CHECK-NEXT:    (ref.null none)
@@ -268,6 +282,12 @@
         (local.get $temp)
       )
     )
+    ;; Signed gets require us to sign-extend them.
+    (drop
+      (struct.get_s $struct.packed 0
+        (local.get $temp)
+      )
+    )
     ;; Unpacked fields in the same struct do not need anything.
     (struct.set $struct.packed 1
       (local.get $temp)
@@ -3409,15 +3429,17 @@
 
 ;; Packed arrays.
 (module
-  ;; CHECK:      (type $0 (func (result i32)))
+  ;; CHECK:      (type $0 (func))
 
   ;; CHECK:      (type $array8 (array (mut i8)))
   (type $array8 (array (mut i8)))
 
+  ;; CHECK:      (type $2 (func (result i32)))
+
   ;; CHECK:      (type $array16 (array (mut i16)))
   (type $array16 (array (mut i16)))
 
-  ;; CHECK:      (func $array8 (type $0) (result i32)
+  ;; CHECK:      (func $array8 (type $0)
   ;; CHECK-NEXT:  (local $temp (ref $array8))
   ;; CHECK-NEXT:  (local $1 i32)
   ;; CHECK-NEXT:  (local $2 i32)
@@ -3447,14 +3469,30 @@
   ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
-  ;; CHECK-NEXT:  (block (result i32)
-  ;; CHECK-NEXT:   (drop
-  ;; CHECK-NEXT:    (ref.null none)
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (block (result i32)
+  ;; CHECK-NEXT:    (drop
+  ;; CHECK-NEXT:     (ref.null none)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (local.get $2)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (block (result i32)
+  ;; CHECK-NEXT:    (drop
+  ;; CHECK-NEXT:     (ref.null none)
+  ;; CHECK-NEXT:    )
+  ;; CHECK-NEXT:    (i32.shr_s
+  ;; CHECK-NEXT:     (i32.shl
+  ;; CHECK-NEXT:      (local.get $2)
+  ;; CHECK-NEXT:      (i32.const 24)
+  ;; CHECK-NEXT:     )
+  ;; CHECK-NEXT:     (i32.const 24)
+  ;; CHECK-NEXT:    )
   ;; CHECK-NEXT:   )
-  ;; CHECK-NEXT:   (local.get $2)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $array8 (result i32)
+  (func $array8
     (local $temp (ref $array8))
     (local.set $temp
       (array.new_default $array8
@@ -3466,13 +3504,22 @@
       (i32.const 1)
       (i32.const 1337)
     )
-    (array.get $array8
-      (local.get $temp)
-      (i32.const 1)
+    (drop
+      (array.get $array8
+        (local.get $temp)
+        (i32.const 1)
+      )
+    )
+    ;; As with structs, a signed get causes us to emit a sign extend.
+    (drop
+      (array.get_s $array8
+        (local.get $temp)
+        (i32.const 1)
+      )
     )
   )
 
-  ;; CHECK:      (func $array16 (type $0) (result i32)
+  ;; CHECK:      (func $array16 (type $2) (result i32)
   ;; CHECK-NEXT:  (local $temp (ref $array16))
   ;; CHECK-NEXT:  (local $1 i32)
   ;; CHECK-NEXT:  (local $2 i32)