[Liftoff] Use ValueKind instead of ValueType

The precise type is only used for validation. For code generation,
knowing the kind is more than enough. Hence, only store and pass the
ValueKind in Liftoff, and not the full ValueType.

R=manoskouk@chromium.org

Bug: v8:11477
Change-Id: Ia42c0fa419f75b508bd2f210c767b631e93d3398
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2707170
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72997}

src/wasm/baseline/liftoff-register.h

@@ -45,12 +45,12 @@ static_assert(kNeedS128RegPair == (kFpRegPair != kNoReg),
 
 enum RegPairHalf : uint8_t { kLowWord = 0, kHighWord = 1 };
 
-static inline constexpr bool needs_gp_reg_pair(ValueType type) {
-  return kNeedI64RegPair && type == kWasmI64;
+static inline constexpr bool needs_gp_reg_pair(ValueKind kind) {
+  return kNeedI64RegPair && kind == kI64;
 }
 
-static inline constexpr bool needs_fp_reg_pair(ValueType type) {
-  return kNeedS128RegPair && type == kWasmS128;
+static inline constexpr bool needs_fp_reg_pair(ValueKind kind) {
+  return kNeedS128RegPair && kind == kS128;
 }
 
 static inline constexpr RegClass reg_class_for(ValueKind kind) {
@@ -72,14 +72,10 @@ static inline constexpr RegClass reg_class_for(ValueKind kind) {
     case kRttWithDepth:
       return kGpReg;
     default:
-      return kNoReg;  // unsupported type
+      return kNoReg;  // unsupported kind
   }
 }
 
-static inline constexpr RegClass reg_class_for(ValueType type) {
-  return reg_class_for(type.kind());
-}
-
 // Description of LiftoffRegister code encoding.
 // This example uses the ARM architecture, which as of writing has:
 // - 9 GP registers, requiring 4 bits
@@ -192,9 +188,9 @@ class LiftoffRegister {
 
   // Shifts the register code depending on the type before converting to a
   // LiftoffRegister.
-  static LiftoffRegister from_external_code(RegClass rc, ValueType type,
+  static LiftoffRegister from_external_code(RegClass rc, ValueKind kind,
                                             int code) {
-    if (!kSimpleFPAliasing && type == kWasmF32) {
+    if (!kSimpleFPAliasing && kind == kF32) {
       // Liftoff assumes a one-to-one mapping between float registers and
       // double registers, and so does not distinguish between f32 and f64
       // registers. The f32 register code must therefore be halved in order
@@ -202,7 +198,7 @@ class LiftoffRegister {
       DCHECK_EQ(0, code % 2);
       return LiftoffRegister::from_code(rc, code >> 1);
     }
-    if (kNeedS128RegPair && type == kWasmS128) {
+    if (kNeedS128RegPair && kind == kS128) {
       // Similarly for double registers and SIMD registers, the SIMD code
       // needs to be doubled to pass the f64 code to Liftoff.
       return LiftoffRegister::ForFpPair(DoubleRegister::from_code(code << 1));

src/wasm/value-type.h

@@ -179,12 +179,94 @@ enum ValueKind : uint8_t {
 #undef DEF_ENUM
 };
 
-// A ValueType is encoded by three components: A Kind, a heap representation
-// (for reference types), and an inheritance depth (for rtts only). Those are
-// encoded into 32 bits using base::BitField. The underlying Kind enumeration
-// includes four elements which do not strictly correspond to value types: the
-// two packed types i8 and i16, the type of void blocks (stmt), and a bottom
-// value (for internal use).
+constexpr bool is_reference_type(ValueKind kind) {
+  return kind == kRef || kind == kOptRef || kind == kRtt ||
+         kind == kRttWithDepth;
+}
+
+constexpr bool is_object_reference_type(ValueKind kind) {
+  return kind == kRef || kind == kOptRef;
+}
+
+constexpr int element_size_log2(ValueKind kind) {
+  constexpr int8_t kElementSizeLog2[] = {
+#define ELEM_SIZE_LOG2(kind, log2Size, ...) log2Size,
+      FOREACH_VALUE_TYPE(ELEM_SIZE_LOG2)
+#undef ELEM_SIZE_LOG2
+  };
+
+  int size_log_2 = kElementSizeLog2[kind];
+  CONSTEXPR_DCHECK(size_log_2 >= 0);
+  return size_log_2;
+}
+
+constexpr int element_size_bytes(ValueKind kind) {
+  constexpr int8_t kElementSize[] = {
+#define ELEM_SIZE_LOG2(kind, log2Size, ...) \
+  log2Size == -1 ? -1 : (1 << std::max(0, log2Size)),
+      FOREACH_VALUE_TYPE(ELEM_SIZE_LOG2)
+#undef ELEM_SIZE_LOG2
+  };
+
+  int size = kElementSize[kind];
+  CONSTEXPR_DCHECK(size > 0);
+  return size;
+}
+
+constexpr char short_name(ValueKind kind) {
+  constexpr char kShortName[] = {
+#define SHORT_NAME(kind, log2Size, code, machineType, shortName, ...) shortName,
+      FOREACH_VALUE_TYPE(SHORT_NAME)
+#undef SHORT_NAME
+  };
+
+  return kShortName[kind];
+}
+
+constexpr const char* name(ValueKind kind) {
+  constexpr const char* kKindName[] = {
+#define KIND_NAME(kind, log2Size, code, machineType, shortName, kindName, ...) \
+  kindName,
+      FOREACH_VALUE_TYPE(KIND_NAME)
+#undef TYPE_NAME
+  };
+
+  return kKindName[kind];
+}
+
+constexpr MachineType machine_type(ValueKind kind) {
+  CONSTEXPR_DCHECK(kBottom != kind);
+
+  constexpr MachineType kMachineType[] = {
+#define MACH_TYPE(kind, log2Size, code, machineType, ...) \
+  MachineType::machineType(),
+      FOREACH_VALUE_TYPE(MACH_TYPE)
+#undef MACH_TYPE
+  };
+
+  return kMachineType[kind];
+}
+
+constexpr bool is_packed(ValueKind kind) { return kind == kI8 || kind == kI16; }
+constexpr ValueKind unpacked(ValueKind kind) {
+  return is_packed(kind) ? kI32 : kind;
+}
+
+constexpr bool is_rtt(ValueKind kind) {
+  return kind == kRtt || kind == kRttWithDepth;
+}
+
+constexpr bool is_defaultable(ValueKind kind) {
+  CONSTEXPR_DCHECK(kind != kBottom && kind != kStmt);
+  return kind != kRef && !is_rtt(kind);
+}
+
+// A ValueType is encoded by three components: A ValueKind, a heap
+// representation (for reference types), and an inheritance depth (for rtts
+// only). Those are encoded into 32 bits using base::BitField. The underlying
+// ValueKind enumeration includes four elements which do not strictly correspond
+// to value types: the two packed types i8 and i16, the type of void blocks
+// (stmt), and a bottom value (for internal use).
 class ValueType {
  public:
   /******************************* Constructors *******************************/
@@ -224,12 +306,11 @@ class ValueType {
 
   /******************************** Type checks *******************************/
   constexpr bool is_reference_type() const {
-    return kind() == kRef || kind() == kOptRef || kind() == kRtt ||
-           kind() == kRttWithDepth;
+    return wasm::is_reference_type(kind());
   }
 
   constexpr bool is_object_reference_type() const {
-    return kind() == kRef || kind() == kOptRef;
+    return wasm::is_object_reference_type(kind());
   }
 
   constexpr bool is_nullable() const { return kind() == kOptRef; }
@@ -239,23 +320,18 @@ class ValueType {
            heap_representation() == htype;
   }
 
-  constexpr bool is_rtt() const {
-    return kind() == kRtt || kind() == kRttWithDepth;
-  }
+  constexpr bool is_rtt() const { return wasm::is_rtt(kind()); }
   constexpr bool has_depth() const { return kind() == kRttWithDepth; }
 
   constexpr bool has_index() const {
     return is_rtt() || (is_object_reference_type() && heap_type().is_index());
   }
 
-  constexpr bool is_defaultable() const {
-    CONSTEXPR_DCHECK(kind() != kBottom && kind() != kStmt);
-    return kind() != kRef && !is_rtt();
-  }
+  constexpr bool is_defaultable() const { return wasm::is_defaultable(kind()); }
 
   constexpr bool is_bottom() const { return kind() == kBottom; }
 
-  constexpr bool is_packed() const { return kind() == kI8 || kind() == kI16; }
+  constexpr bool is_packed() const { return wasm::is_packed(kind()); }
 
   constexpr ValueType Unpacked() const {
     return is_packed() ? Primitive(kI32) : *this;
@@ -301,42 +377,16 @@ class ValueType {
   }
 
   constexpr int element_size_log2() const {
-    constexpr int8_t kElementSizeLog2[] = {
-#define ELEM_SIZE_LOG2(kind, log2Size, ...) log2Size,
-        FOREACH_VALUE_TYPE(ELEM_SIZE_LOG2)
-#undef ELEM_SIZE_LOG2
-    };
-
-    int size_log_2 = kElementSizeLog2[kind()];
-    CONSTEXPR_DCHECK(size_log_2 >= 0);
-    return size_log_2;
+    return wasm::element_size_log2(kind());
   }
 
   constexpr int element_size_bytes() const {
-    constexpr int8_t kElementSize[] = {
-#define ELEM_SIZE_LOG2(kind, log2Size, ...) \
-  log2Size == -1 ? -1 : (1 << std::max(0, log2Size)),
-        FOREACH_VALUE_TYPE(ELEM_SIZE_LOG2)
-#undef ELEM_SIZE_LOG2
-    };
-
-    int size = kElementSize[kind()];
-    CONSTEXPR_DCHECK(size > 0);
-    return size;
+    return wasm::element_size_bytes(kind());
   }
 
   /*************************** Machine-type related ***************************/
   constexpr MachineType machine_type() const {
-    CONSTEXPR_DCHECK(kBottom != kind());
-
-    constexpr MachineType kMachineType[] = {
-#define MACH_TYPE(kind, log2Size, code, machineType, ...) \
-  MachineType::machineType(),
-        FOREACH_VALUE_TYPE(MACH_TYPE)
-#undef MACH_TYPE
-    };
-
-    return kMachineType[kind()];
+    return wasm::machine_type(kind());
   }
 
   constexpr MachineRepresentation machine_representation() const {
@@ -427,15 +477,7 @@ class ValueType {
   static constexpr int kLastUsedBit = 30;
 
   /****************************** Pretty-printing *****************************/
-  constexpr char short_name() const {
-    constexpr char kShortName[] = {
-#define SHORT_NAME(kind, log2Size, code, machineType, shortName, ...) shortName,
-        FOREACH_VALUE_TYPE(SHORT_NAME)
-#undef SHORT_NAME
-    };
-
-    return kShortName[kind()];
-  }
+  constexpr char short_name() const { return wasm::short_name(kind()); }
 
   std::string name() const {
     std::ostringstream buf;
@@ -483,16 +525,7 @@ class ValueType {
 
   constexpr explicit ValueType(uint32_t bit_field) : bit_field_(bit_field) {}
 
-  constexpr const char* kind_name() const {
-    constexpr const char* kTypeName[] = {
-#define KIND_NAME(kind, log2Size, code, machineType, shortName, typeName, ...) \
-  typeName,
-        FOREACH_VALUE_TYPE(KIND_NAME)
-#undef TYPE_NAME
-    };
-
-    return kTypeName[kind()];
-  }
+  constexpr const char* kind_name() const { return wasm::name(kind()); }
 
   uint32_t bit_field_;
 };
@@ -573,8 +606,8 @@ class LoadType {
   constexpr ValueType value_type() const { return kValueType[val_]; }
   constexpr MachineType mem_type() const { return kMemType[val_]; }
 
-  static LoadType ForValueType(ValueType type, bool is_signed = false) {
-    switch (type.kind()) {
+  static LoadType ForValueKind(ValueKind kind, bool is_signed = false) {
+    switch (kind) {
       case kI32:
         return kI32Load;
       case kI64:
@@ -649,8 +682,8 @@ class StoreType {
   constexpr ValueType value_type() const { return kValueType[val_]; }
   constexpr MachineRepresentation mem_rep() const { return kMemRep[val_]; }
 
-  static StoreType ForValueType(ValueType type) {
-    switch (type.kind()) {
+  static StoreType ForValueKind(ValueKind kind) {
+    switch (kind) {
       case kI32:
         return kI32Store;
       case kI64:

src/wasm/wasm-debug.cc

@@ -95,8 +95,8 @@ void DebugSideTable::Entry::Print(std::ostream& os) const {
   os << std::setw(6) << std::hex << pc_offset_ << std::dec << " stack height "
      << stack_height_ << " [";
   for (auto& value : changed_values_) {
-    os << " " << value.type.name() << ":";
-    switch (value.kind) {
+    os << " " << name(value.kind) << ":";
+    switch (value.storage) {
       case kConstant:
         os << "const#" << value.i32_const;
         break;
@@ -510,8 +510,8 @@ class DebugInfoImpl {
     const auto* value =
         debug_side_table->FindValue(debug_side_table_entry, index);
     if (value->is_constant()) {
-      DCHECK(value->type == kWasmI32 || value->type == kWasmI64);
-      return value->type == kWasmI32 ? WasmValue(value->i32_const)
+      DCHECK(value->kind == kI32 || value->kind == kI64);
+      return value->kind == kI32 ? WasmValue(value->i32_const)
                                  : WasmValue(int64_t{value->i32_const});
     }
 
@@ -523,14 +523,14 @@ class DebugInfoImpl {
                    reg.code());
       };
       if (reg.is_gp_pair()) {
-        DCHECK_EQ(kWasmI64, value->type);
+        DCHECK_EQ(kI64, value->kind);
         uint32_t low_word = ReadUnalignedValue<uint32_t>(gp_addr(reg.low_gp()));
         uint32_t high_word =
             ReadUnalignedValue<uint32_t>(gp_addr(reg.high_gp()));
         return WasmValue((uint64_t{high_word} << 32) | low_word);
       }
       if (reg.is_gp()) {
-        return value->type == kWasmI32
+        return value->kind == kI32
                    ? WasmValue(ReadUnalignedValue<uint32_t>(gp_addr(reg.gp())))
                    : WasmValue(ReadUnalignedValue<uint64_t>(gp_addr(reg.gp())));
       }
@@ -544,11 +544,11 @@ class DebugInfoImpl {
       Address spilled_addr =
           debug_break_fp +
           WasmDebugBreakFrameConstants::GetPushedFpRegisterOffset(code);
-      if (value->type == kWasmF32) {
+      if (value->kind == kF32) {
         return WasmValue(ReadUnalignedValue<float>(spilled_addr));
-      } else if (value->type == kWasmF64) {
+      } else if (value->kind == kF64) {
         return WasmValue(ReadUnalignedValue<double>(spilled_addr));
-      } else if (value->type == kWasmS128) {
+      } else if (value->kind == kS128) {
         return WasmValue(Simd128(ReadUnalignedValue<int16>(spilled_addr)));
       } else {
         // All other cases should have been handled above.
@@ -558,7 +558,7 @@ class DebugInfoImpl {
 
     // Otherwise load the value from the stack.
     Address stack_address = stack_frame_base - value->stack_offset;
-    switch (value->type.kind()) {
+    switch (value->kind) {
       case kI32:
         return WasmValue(ReadUnalignedValue<int32_t>(stack_address));
       case kI64:

src/wasm/wasm-debug.h

@@ -40,11 +40,11 @@ class DebugSideTable {
  public:
   class Entry {
    public:
-    enum ValueKind : int8_t { kConstant, kRegister, kStack };
+    enum Storage : int8_t { kConstant, kRegister, kStack };
     struct Value {
       int index;
-      ValueType type;
       ValueKind kind;
+      Storage storage;
       union {
         int32_t i32_const;  // if kind == kConstant
         int reg_code;       // if kind == kRegister
@@ -53,9 +53,9 @@ class DebugSideTable {
 
       bool operator==(const Value& other) const {
         if (index != other.index) return false;
-        if (type != other.type) return false;
         if (kind != other.kind) return false;
-        switch (kind) {
+        if (storage != other.storage) return false;
+        switch (storage) {
           case kConstant:
             return i32_const == other.i32_const;
           case kRegister:
@@ -66,8 +66,8 @@ class DebugSideTable {
       }
       bool operator!=(const Value& other) const { return !(*this == other); }
 
-      bool is_constant() const { return kind == kConstant; }
-      bool is_register() const { return kind == kRegister; }
+      bool is_constant() const { return storage == kConstant; }
+      bool is_register() const { return storage == kRegister; }
     };
 
     Entry(int pc_offset, int stack_height, std::vector<Value> changed_values)

test/cctest/wasm/test-liftoff-inspection.cc

@@ -177,8 +177,8 @@ struct DebugSideTableEntry {
   // Check for equality, but ignore exact register and stack offset.
   static bool CheckValueEquals(const DebugSideTable::Entry::Value& a,
                                const DebugSideTable::Entry::Value& b) {
-    return a.index == b.index && a.type == b.type && a.kind == b.kind &&
-           (a.kind != DebugSideTable::Entry::kConstant ||
+    return a.index == b.index && a.kind == b.kind && a.kind == b.kind &&
+           (a.storage != DebugSideTable::Entry::kConstant ||
             a.i32_const == b.i32_const);
   }
 };
@@ -189,8 +189,8 @@ std::ostream& operator<<(std::ostream& out, const DebugSideTableEntry& entry) {
   out << "stack height " << entry.stack_height << ", changed: {";
   const char* comma = "";
   for (auto& v : entry.changed_values) {
-    out << comma << v.index << ":" << v.type.name() << " ";
-    switch (v.kind) {
+    out << comma << v.index << ":" << name(v.kind) << " ";
+    switch (v.storage) {
       case DebugSideTable::Entry::kConstant:
         out << "const:" << v.i32_const;
         break;
@@ -213,27 +213,27 @@ std::ostream& operator<<(std::ostream& out,
 #endif  // DEBUG
 
 // Named constructors to make the tests more readable.
-DebugSideTable::Entry::Value Constant(int index, ValueType type,
+DebugSideTable::Entry::Value Constant(int index, ValueKind kind,
                                       int32_t constant) {
   DebugSideTable::Entry::Value value;
   value.index = index;
-  value.type = type;
-  value.kind = DebugSideTable::Entry::kConstant;
+  value.kind = kind;
+  value.storage = DebugSideTable::Entry::kConstant;
   value.i32_const = constant;
   return value;
 }
-DebugSideTable::Entry::Value Register(int index, ValueType type) {
+DebugSideTable::Entry::Value Register(int index, ValueKind kind) {
   DebugSideTable::Entry::Value value;
   value.index = index;
-  value.type = type;
-  value.kind = DebugSideTable::Entry::kRegister;
+  value.kind = kind;
+  value.storage = DebugSideTable::Entry::kRegister;
   return value;
 }
-DebugSideTable::Entry::Value Stack(int index, ValueType type) {
+DebugSideTable::Entry::Value Stack(int index, ValueKind kind) {
   DebugSideTable::Entry::Value value;
   value.index = index;
-  value.type = type;
-  value.kind = DebugSideTable::Entry::kStack;
+  value.kind = kind;
+  value.storage = DebugSideTable::Entry::kStack;
   return value;
 }
 
@@ -296,9 +296,9 @@ TEST(Liftoff_debug_side_table_simple) {
   CheckDebugSideTable(
       {
           // function entry, locals in registers.
-          {2, {Register(0, kWasmI32), Register(1, kWasmI32)}},
+          {2, {Register(0, kI32), Register(1, kI32)}},
           // OOL stack check, locals spilled, stack still empty.
-          {2, {Stack(0, kWasmI32), Stack(1, kWasmI32)}},
+          {2, {Stack(0, kI32), Stack(1, kI32)}},
       },
       debug_side_table.get());
 }
@@ -312,9 +312,9 @@ TEST(Liftoff_debug_side_table_call) {
   CheckDebugSideTable(
       {
           // function entry, local in register.
-          {1, {Register(0, kWasmI32)}},
+          {1, {Register(0, kI32)}},
           // call, local spilled, stack empty.
-          {1, {Stack(0, kWasmI32)}},
+          {1, {Stack(0, kI32)}},
           // OOL stack check, local spilled as before, stack empty.
           {1, {}},
       },
@@ -332,11 +332,11 @@ TEST(Liftoff_debug_side_table_call_const) {
   CheckDebugSideTable(
       {
           // function entry, local in register.
-          {1, {Register(0, kWasmI32)}},
+          {1, {Register(0, kI32)}},
           // call, local is kConst.
-          {1, {Constant(0, kWasmI32, kConst)}},
+          {1, {Constant(0, kI32, kConst)}},
           // OOL stack check, local spilled.
-          {1, {Stack(0, kWasmI32)}},
+          {1, {Stack(0, kI32)}},
       },
       debug_side_table.get());
 }
@@ -351,13 +351,13 @@ TEST(Liftoff_debug_side_table_indirect_call) {
   CheckDebugSideTable(
       {
           // function entry, local in register.
-          {1, {Register(0, kWasmI32)}},
+          {1, {Register(0, kI32)}},
           // indirect call, local spilled, stack empty.
-          {1, {Stack(0, kWasmI32)}},
+          {1, {Stack(0, kI32)}},
           // OOL stack check, local still spilled.
           {1, {}},
           // OOL trap (invalid index), local still spilled, stack has {kConst}.
-          {2, {Constant(1, kWasmI32, kConst)}},
+          {2, {Constant(1, kI32, kConst)}},
           // OOL trap (sig mismatch), stack unmodified.
           {2, {}},
       },
@@ -373,11 +373,11 @@ TEST(Liftoff_debug_side_table_loop) {
   CheckDebugSideTable(
       {
           // function entry, local in register.
-          {1, {Register(0, kWasmI32)}},
+          {1, {Register(0, kI32)}},
           // OOL stack check, local spilled, stack empty.
-          {1, {Stack(0, kWasmI32)}},
+          {1, {Stack(0, kI32)}},
           // OOL loop stack check, local still spilled, stack has {kConst}.
-          {2, {Constant(1, kWasmI32, kConst)}},
+          {2, {Constant(1, kI32, kConst)}},
       },
       debug_side_table.get());
 }
@@ -390,9 +390,9 @@ TEST(Liftoff_debug_side_table_trap) {
   CheckDebugSideTable(
       {
           // function entry, locals in registers.
-          {2, {Register(0, kWasmI32), Register(1, kWasmI32)}},
+          {2, {Register(0, kI32), Register(1, kI32)}},
           // OOL stack check, local spilled, stack empty.
-          {2, {Stack(0, kWasmI32), Stack(1, kWasmI32)}},
+          {2, {Stack(0, kI32), Stack(1, kI32)}},
           // OOL trap (div by zero), stack as before.
           {2, {}},
           // OOL trap (unrepresentable), stack as before.
@@ -414,11 +414,11 @@ TEST(Liftoff_breakpoint_simple) {
   CheckDebugSideTable(
       {
           // First break point, locals in registers.
-          {2, {Register(0, kWasmI32), Register(1, kWasmI32)}},
+          {2, {Register(0, kI32), Register(1, kI32)}},
           // Second break point, locals unchanged, two register stack values.
-          {4, {Register(2, kWasmI32), Register(3, kWasmI32)}},
+          {4, {Register(2, kI32), Register(3, kI32)}},
           // OOL stack check, locals spilled, stack empty.
-          {2, {Stack(0, kWasmI32), Stack(1, kWasmI32)}},
+          {2, {Stack(0, kI32), Stack(1, kI32)}},
       },
       debug_side_table.get());
 }