[Liftoff] Store ValueType in each VarState

In order to implement CacheState merging correctly, we need to know at
least the register type to be used for each stack slot. For stack
slots, this is not stored currently. Since there is enough space
available in the VarState anyway, we just always store the full type,
which will allow also for other optimizations like not always spilling
and filling the full 8 bytes.

R=titzer@chromium.org

Bug: v8:6600
Change-Id: I6c7c1d39847063591bf72b7f186a2128295d889b
Reviewed-on: https://chromium-review.googlesource.com/789861
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49641}

src/wasm/baseline/liftoff-assembler.cc

@@ -185,7 +185,7 @@ void LiftoffAssembler::CacheState::InitMerge(const CacheState& source,
                                              uint32_t arity) {
   DCHECK(stack_state.empty());
   DCHECK_GE(source.stack_height(), stack_base);
-  stack_state.resize(stack_base + arity);
+  stack_state.resize(stack_base + arity, VarState(kWasmStmt));
 
   // |------locals------|--(in between)--|--(discarded)--|----merge----|
   //  <-- num_locals -->                 ^stack_base      <-- arity -->
@@ -206,12 +206,12 @@ void LiftoffAssembler::CacheState::InitMerge(const CacheState& source,
       } else if (has_unused_register()) {
         reg = unused_register();
       } else {
-        // Keep this a stack slot (which is the initial value).
+        // Make this a stack slot.
         DCHECK(src.is_stack());
-        DCHECK(dst.is_stack());
+        dst = VarState(src.type());
         continue;
       }
-      dst = VarState(reg);
+      dst = VarState(src.type(), reg);
       inc_used(reg);
     }
   }
@@ -223,11 +223,11 @@ void LiftoffAssembler::CacheState::InitMerge(const CacheState& source,
     auto& src = source.stack_state[i];
     if (src.is_reg()) {
       if (is_used(src.reg())) {
-        // Keep this a stack slot (which is the initial value).
-        DCHECK(dst.is_stack());
+        // Make this a stack slot.
+        dst = VarState(src.type());
         continue;
       }
-      dst = VarState(src.reg());
+      dst = VarState(src.type(), src.reg());
       inc_used(src.reg());
     } else if (src.is_const()) {
       dst = src;

src/wasm/baseline/liftoff-assembler.h

@@ -80,10 +80,13 @@ class LiftoffAssembler : public TurboAssembler {
    public:
     enum Location : uint8_t { kStack, kRegister, kConstant };
 
-    VarState() : loc_(kStack) {}
-    explicit VarState(Register r) : loc_(kRegister), reg_(r) {}
-    explicit VarState(uint32_t i32_const)
-        : loc_(kConstant), i32_const_(i32_const) {}
+    explicit VarState(ValueType type) : loc_(kStack), type_(type) {}
+    explicit VarState(ValueType type, Register r)
+        : loc_(kRegister), type_(type), reg_(r) {}
+    explicit VarState(ValueType type, uint32_t i32_const)
+        : loc_(kConstant), type_(type), i32_const_(i32_const) {
+      DCHECK(type_ == kWasmI32 || type_ == kWasmI64);
+    }
 
     bool operator==(const VarState& other) const {
       if (loc_ != other.loc_) return false;
@@ -102,6 +105,8 @@ class LiftoffAssembler : public TurboAssembler {
     bool is_reg() const { return loc_ == kRegister; }
     bool is_const() const { return loc_ == kConstant; }
 
+    ValueType type() const { return type_; }
+
     Location loc() const { return loc_; }
 
     uint32_t i32_const() const {
@@ -118,6 +123,9 @@ class LiftoffAssembler : public TurboAssembler {
 
    private:
     Location loc_;
+    // TODO(wasm): This is redundant, the decoder already knows the type of each
+    // stack value. Try to collapse.
+    ValueType type_;
 
     union {
       Register reg_;        // used if loc_ == kRegister
@@ -220,9 +228,9 @@ class LiftoffAssembler : public TurboAssembler {
 
   Register PopToRegister(ValueType, PinnedRegisterScope = {});
 
-  void PushRegister(Register reg) {
+  void PushRegister(ValueType type, Register reg) {
     cache_state_.inc_used(reg);
-    cache_state_.stack_state.emplace_back(reg);
+    cache_state_.stack_state.emplace_back(type, reg);
   }
 
   uint32_t GetNumUses(Register reg) const {

src/wasm/baseline/liftoff-compiler.cc

@@ -141,6 +141,7 @@ class LiftoffCompiler {
     uint32_t param_idx = 0;
     for (; param_idx < num_params; ++param_idx) {
       constexpr uint32_t kFirstActualParamIndex = kContextParameterIndex + 1;
+      ValueType type = __ local_type(param_idx);
       compiler::LinkageLocation param_loc =
           call_desc_->GetInputLocation(param_idx + kFirstActualParamIndex);
       if (param_loc.IsRegister()) {
@@ -148,16 +149,16 @@ class LiftoffCompiler {
         Register param_reg = Register::from_code(param_loc.AsRegister());
         if (param_reg.bit() & __ kGpCacheRegs) {
           // This is a cache register, just use it.
-          __ PushRegister(param_reg);
+          __ PushRegister(type, param_reg);
         } else {
           // No cache register. Push to the stack.
           __ Spill(param_idx, param_reg);
-          __ cache_state()->stack_state.emplace_back();
+          __ cache_state()->stack_state.emplace_back(type);
         }
       } else if (param_loc.IsCallerFrameSlot()) {
-        Register tmp_reg = __ GetUnusedRegister(__ local_type(param_idx));
+        Register tmp_reg = __ GetUnusedRegister(type);
         __ LoadCallerFrameSlot(tmp_reg, -param_loc.AsCallerFrameSlot());
-        __ PushRegister(tmp_reg);
+        __ PushRegister(type, tmp_reg);
       } else {
         UNIMPLEMENTED();
       }
@@ -166,7 +167,7 @@ class LiftoffCompiler {
       ValueType type = decoder->GetLocalType(param_idx);
       switch (type) {
         case kWasmI32:
-          __ cache_state()->stack_state.emplace_back(0);
+          __ cache_state()->stack_state.emplace_back(kWasmI32, uint32_t{0});
           break;
         default:
           UNIMPLEMENTED();
@@ -263,11 +264,11 @@ class LiftoffCompiler {
     Register rhs_reg = pinned_regs.pin(__ PopToRegister(kWasmI32, pinned_regs));
     Register lhs_reg = __ PopToRegister(kWasmI32, pinned_regs);
     (asm_->*emit_fn)(target_reg, lhs_reg, rhs_reg);
-    __ PushRegister(target_reg);
+    __ PushRegister(kWasmI32, target_reg);
   }
 
   void I32Const(Decoder* decoder, Value* result, int32_t value) {
-    __ cache_state()->stack_state.emplace_back(value);
+    __ cache_state()->stack_state.emplace_back(kWasmI32, value);
     CheckStackSizeLimit(decoder);
   }
 
@@ -310,15 +311,16 @@ class LiftoffCompiler {
     auto& slot = __ cache_state()->stack_state[operand.index];
     switch (slot.loc()) {
       case kRegister:
-        __ PushRegister(slot.reg());
+        __ PushRegister(operand.type, slot.reg());
         break;
       case kConstant:
-        __ cache_state()->stack_state.emplace_back(slot.i32_const());
+        __ cache_state()->stack_state.emplace_back(operand.type,
+                                                   slot.i32_const());
         break;
       case kStack: {
         Register reg = __ GetUnusedRegister(__ local_type(operand.index));
         __ Fill(reg, operand.index);
-        __ PushRegister(reg);
+        __ PushRegister(operand.type, reg);
       } break;
     }
     CheckStackSizeLimit(decoder);
@@ -350,10 +352,11 @@ class LiftoffCompiler {
             }
           case kConstant:
           case kStack: {
+            ValueType type = __ local_type(local_index);
             Register target_reg =
                 __ GetUnusedRegister(__ local_type(local_index));
             __ Fill(target_reg, state.stack_height() - 1);
-            target_slot = LiftoffAssembler::VarState(target_reg);
+            target_slot = LiftoffAssembler::VarState(type, target_reg);
             state.inc_used(target_reg);
           } break;
         }
@@ -387,7 +390,7 @@ class LiftoffCompiler {
     if (size > kPointerSize)
       return unsupported(decoder, "global > kPointerSize");
     __ Load(value, addr, global->offset, size, pinned);
-    __ PushRegister(value);
+    __ PushRegister(global->type, value);
   }
 
   void SetGlobal(Decoder* decoder, const Value& value,