tint/ir: Remove 'type' field from Instruction

ir::Value has a virtual Type() method. Implement this on the subclasses
that actually have a type.

Removed pointless memory usage on instructions that have no value.

Also un-getter some fields to match the other IR code.

Change-Id: Ibf545a582d5cbb820410c9aaec0312692e803373
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/131745
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/tint/ir/binary.cc

@@ -19,10 +19,10 @@ TINT_INSTANTIATE_TYPEINFO(tint::ir::Binary);
 
 namespace tint::ir {
 
-Binary::Binary(Kind kind, const type::Type* ty, Value* lhs, Value* rhs)
+Binary::Binary(Kind k, const type::Type* res_ty, Value* lhs, Value* rhs)
-    : Base(ty), kind_(kind), lhs_(lhs), rhs_(rhs) {
+    : kind(k), result_type(res_ty), lhs_(lhs), rhs_(rhs) {
-    TINT_ASSERT(IR, lhs_);
+    TINT_ASSERT(IR, lhs);
-    TINT_ASSERT(IR, rhs_);
+    TINT_ASSERT(IR, rhs);
     lhs_->AddUsage(this);
     rhs_->AddUsage(this);
 }

src/tint/ir/binary.h

@@ -59,8 +59,8 @@ class Binary : public utils::Castable<Binary, Instruction> {
     Binary& operator=(const Binary& inst) = delete;
     Binary& operator=(Binary&& inst) = delete;
 
-    /// @returns the kind of instruction
+    /// @returns the type of the value
-    Kind GetKind() const { return kind_; }
+    const type::Type* Type() const override { return result_type; }
 
     /// @returns the left-hand-side value for the instruction
     const Value* LHS() const { return lhs_; }
@@ -68,8 +68,13 @@ class Binary : public utils::Castable<Binary, Instruction> {
     /// @returns the right-hand-side value for the instruction
     const Value* RHS() const { return rhs_; }
 
+    /// the kind of binary instruction
+    Kind kind = Kind::kAdd;
+
+    /// the result type of the instruction
+    const type::Type* result_type = nullptr;
+
   private:
-    Kind kind_;
     Value* lhs_ = nullptr;
     Value* rhs_ = nullptr;
 };

src/tint/ir/binary_test.cc

@@ -29,7 +29,8 @@ TEST_F(IR_InstructionTest, CreateAnd) {
                                      b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kAnd);
+    EXPECT_EQ(inst->kind, Binary::Kind::kAnd);
+    ASSERT_NE(inst->result_type, nullptr);
     ASSERT_NE(inst->Type(), nullptr);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
@@ -50,7 +51,7 @@ TEST_F(IR_InstructionTest, CreateOr) {
                                     b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kOr);
+    EXPECT_EQ(inst->kind, Binary::Kind::kOr);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -70,7 +71,7 @@ TEST_F(IR_InstructionTest, CreateXor) {
                                      b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kXor);
+    EXPECT_EQ(inst->kind, Binary::Kind::kXor);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -90,7 +91,7 @@ TEST_F(IR_InstructionTest, CreateEqual) {
                                        b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kEqual);
+    EXPECT_EQ(inst->kind, Binary::Kind::kEqual);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -110,7 +111,7 @@ TEST_F(IR_InstructionTest, CreateNotEqual) {
                                           b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kNotEqual);
+    EXPECT_EQ(inst->kind, Binary::Kind::kNotEqual);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -130,7 +131,7 @@ TEST_F(IR_InstructionTest, CreateLessThan) {
                                           b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kLessThan);
+    EXPECT_EQ(inst->kind, Binary::Kind::kLessThan);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -150,7 +151,7 @@ TEST_F(IR_InstructionTest, CreateGreaterThan) {
                                              b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kGreaterThan);
+    EXPECT_EQ(inst->kind, Binary::Kind::kGreaterThan);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -170,7 +171,7 @@ TEST_F(IR_InstructionTest, CreateLessThanEqual) {
                                                b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kLessThanEqual);
+    EXPECT_EQ(inst->kind, Binary::Kind::kLessThanEqual);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -190,7 +191,7 @@ TEST_F(IR_InstructionTest, CreateGreaterThanEqual) {
                                                   b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kGreaterThanEqual);
+    EXPECT_EQ(inst->kind, Binary::Kind::kGreaterThanEqual);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -209,7 +210,7 @@ TEST_F(IR_InstructionTest, CreateNot) {
         b.builder.Not(b.builder.ir.types.Get<type::Bool>(), b.builder.Constant(true));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kEqual);
+    EXPECT_EQ(inst->kind, Binary::Kind::kEqual);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -229,7 +230,7 @@ TEST_F(IR_InstructionTest, CreateShiftLeft) {
                                            b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kShiftLeft);
+    EXPECT_EQ(inst->kind, Binary::Kind::kShiftLeft);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -249,7 +250,7 @@ TEST_F(IR_InstructionTest, CreateShiftRight) {
                                             b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kShiftRight);
+    EXPECT_EQ(inst->kind, Binary::Kind::kShiftRight);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -269,7 +270,7 @@ TEST_F(IR_InstructionTest, CreateAdd) {
                                      b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kAdd);
+    EXPECT_EQ(inst->kind, Binary::Kind::kAdd);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -289,7 +290,7 @@ TEST_F(IR_InstructionTest, CreateSubtract) {
                                           b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kSubtract);
+    EXPECT_EQ(inst->kind, Binary::Kind::kSubtract);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -309,7 +310,7 @@ TEST_F(IR_InstructionTest, CreateMultiply) {
                                           b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kMultiply);
+    EXPECT_EQ(inst->kind, Binary::Kind::kMultiply);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -329,7 +330,7 @@ TEST_F(IR_InstructionTest, CreateDivide) {
                                         b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kDivide);
+    EXPECT_EQ(inst->kind, Binary::Kind::kDivide);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -349,7 +350,7 @@ TEST_F(IR_InstructionTest, CreateModulo) {
                                         b.builder.Constant(4_i), b.builder.Constant(2_i));
 
     ASSERT_TRUE(inst->Is<Binary>());
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kModulo);
+    EXPECT_EQ(inst->kind, Binary::Kind::kModulo);
 
     ASSERT_TRUE(inst->LHS()->Is<Constant>());
     auto lhs = inst->LHS()->As<Constant>()->value;
@@ -367,7 +368,7 @@ TEST_F(IR_InstructionTest, Binary_Usage) {
     const auto* inst = b.builder.And(b.builder.ir.types.Get<type::I32>(), b.builder.Constant(4_i),
                                      b.builder.Constant(2_i));
 
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kAnd);
+    EXPECT_EQ(inst->kind, Binary::Kind::kAnd);
 
     ASSERT_NE(inst->LHS(), nullptr);
     ASSERT_EQ(inst->LHS()->Usage().Length(), 1u);
@@ -383,7 +384,7 @@ TEST_F(IR_InstructionTest, Binary_Usage_DuplicateValue) {
     auto val = b.builder.Constant(4_i);
     const auto* inst = b.builder.And(b.builder.ir.types.Get<type::I32>(), val, val);
 
-    EXPECT_EQ(inst->GetKind(), Binary::Kind::kAnd);
+    EXPECT_EQ(inst->kind, Binary::Kind::kAnd);
     ASSERT_EQ(inst->LHS(), inst->RHS());
 
     ASSERT_NE(inst->LHS(), nullptr);

src/tint/ir/builtin.h

@@ -25,10 +25,10 @@ namespace tint::ir {
 class Builtin : public utils::Castable<Builtin, Call> {
   public:
     /// Constructor
-    /// @param type the result type
+    /// @param res_type the result type
     /// @param func the builtin function
     /// @param args the conversion arguments
-    Builtin(const type::Type* type, builtin::Function func, utils::VectorRef<Value*> args);
+    Builtin(const type::Type* res_type, builtin::Function func, utils::VectorRef<Value*> args);
     Builtin(const Builtin& inst) = delete;
     Builtin(Builtin&& inst) = delete;
     ~Builtin() override;

src/tint/ir/call.cc

@@ -20,8 +20,8 @@ TINT_INSTANTIATE_TYPEINFO(tint::ir::Call);
 
 namespace tint::ir {
 
-Call::Call(const type::Type* ty, utils::VectorRef<Value*> arguments)
+Call::Call(const type::Type* res_ty, utils::VectorRef<Value*> arguments)
-    : Base(ty), args(std::move(arguments)) {
+    : result_type(res_ty), args(std::move(arguments)) {
     for (auto* arg : args) {
         arg->AddUsage(this);
     }

src/tint/ir/call.h

@@ -30,6 +30,12 @@ class Call : public utils::Castable<Call, Instruction> {
     Call& operator=(const Call& inst) = delete;
     Call& operator=(Call&& inst) = delete;
 
+    /// @returns the type of the value
+    const type::Type* Type() const override { return result_type; }
+
+    /// The instruction type
+    const type::Type* result_type = nullptr;
+
     /// The constructor arguments
     utils::Vector<Value*, 1> args;
 
@@ -37,9 +43,9 @@ class Call : public utils::Castable<Call, Instruction> {
     /// Constructor
     Call() = delete;
     /// Constructor
-    /// @param type the result type
+    /// @param result_type the result type
     /// @param args the constructor arguments
-    Call(const type::Type* type, utils::VectorRef<Value*> args);
+    Call(const type::Type* result_type, utils::VectorRef<Value*> args);
 };
 
 }  // namespace tint::ir

src/tint/ir/disassembler.cc

@@ -428,7 +428,7 @@ void Disassembler::EmitArgs(const Call* call) {
 void Disassembler::EmitBinary(const Binary* b) {
     EmitValue(b);
     out_ << " = ";
-    switch (b->GetKind()) {
+    switch (b->kind) {
         case Binary::Kind::kAdd:
             out_ << "add";
             break;
@@ -487,7 +487,7 @@ void Disassembler::EmitBinary(const Binary* b) {
 void Disassembler::EmitUnary(const Unary* u) {
     EmitValue(u);
     out_ << " = ";
-    switch (u->GetKind()) {
+    switch (u->kind) {
         case Unary::Kind::kAddressOf:
             out_ << "addr_of";
             break;

src/tint/ir/instruction.cc

@@ -20,8 +20,6 @@ namespace tint::ir {
 
 Instruction::Instruction() = default;
 
-Instruction::Instruction(const type::Type* ty) : type(ty) {}
-
 Instruction::~Instruction() = default;
 
 }  // namespace tint::ir

src/tint/ir/instruction.h

@@ -31,18 +31,9 @@ class Instruction : public utils::Castable<Instruction, Value> {
     Instruction& operator=(const Instruction& inst) = delete;
     Instruction& operator=(Instruction&& inst) = delete;
 
-    /// @returns the type of the value
-    const type::Type* Type() const override { return type; }
-
-    /// The instruction type
-    const type::Type* type = nullptr;
-
   protected:
     /// Constructor
     Instruction();
-    /// Constructor
-    /// @param type the result type
-    explicit Instruction(const type::Type* type);
 };
 
 }  // namespace tint::ir

src/tint/ir/unary.cc

@@ -19,7 +19,8 @@ TINT_INSTANTIATE_TYPEINFO(tint::ir::Unary);
 
 namespace tint::ir {
 
-Unary::Unary(Kind kind, const type::Type* ty, Value* val) : Base(ty), kind_(kind), val_(val) {
+Unary::Unary(Kind k, const type::Type* res_ty, Value* val)
+    : kind(k), result_type(res_ty), val_(val) {
     TINT_ASSERT(IR, val_);
     val_->AddUsage(this);
 }

src/tint/ir/unary.h

@@ -33,9 +33,9 @@ class Unary : public utils::Castable<Unary, Instruction> {
 
     /// Constructor
     /// @param kind the kind of unary instruction
-    /// @param type the result type
+    /// @param result_type the result type
-    /// @param val the lhs of the instruction
+    /// @param val the input value for the instruction
-    Unary(Kind kind, const type::Type* type, Value* val);
+    Unary(Kind kind, const type::Type* result_type, Value* val);
     Unary(const Unary& inst) = delete;
     Unary(Unary&& inst) = delete;
     ~Unary() override;
@@ -43,14 +43,19 @@ class Unary : public utils::Castable<Unary, Instruction> {
     Unary& operator=(const Unary& inst) = delete;
     Unary& operator=(Unary&& inst) = delete;
 
-    /// @returns the kind of instruction
+    /// @returns the type of the value
-    Kind GetKind() const { return kind_; }
+    const type::Type* Type() const override { return result_type; }
 
     /// @returns the value for the instruction
     const Value* Val() const { return val_; }
 
+    /// the kind of unary instruction
+    Kind kind = Kind::kAddressOf;
+
+    /// the result type of the instruction
+    const type::Type* result_type = nullptr;
+
   private:
-    Kind kind_;
     Value* val_ = nullptr;
 };
 

src/tint/ir/unary_test.cc

@@ -33,7 +33,7 @@ TEST_F(IR_InstructionTest, CreateAddressOf) {
                             b.builder.Constant(4_i));
 
     ASSERT_TRUE(inst->Is<Unary>());
-    EXPECT_EQ(inst->GetKind(), Unary::Kind::kAddressOf);
+    EXPECT_EQ(inst->kind, Unary::Kind::kAddressOf);
 
     ASSERT_NE(inst->Type(), nullptr);
 
@@ -49,7 +49,7 @@ TEST_F(IR_InstructionTest, CreateComplement) {
         b.builder.Complement(b.builder.ir.types.Get<type::I32>(), b.builder.Constant(4_i));
 
     ASSERT_TRUE(inst->Is<Unary>());
-    EXPECT_EQ(inst->GetKind(), Unary::Kind::kComplement);
+    EXPECT_EQ(inst->kind, Unary::Kind::kComplement);
 
     ASSERT_TRUE(inst->Val()->Is<Constant>());
     auto lhs = inst->Val()->As<Constant>()->value;
@@ -65,7 +65,7 @@ TEST_F(IR_InstructionTest, CreateIndirection) {
         b.builder.Indirection(b.builder.ir.types.Get<type::I32>(), b.builder.Constant(4_i));
 
     ASSERT_TRUE(inst->Is<Unary>());
-    EXPECT_EQ(inst->GetKind(), Unary::Kind::kIndirection);
+    EXPECT_EQ(inst->kind, Unary::Kind::kIndirection);
 
     ASSERT_TRUE(inst->Val()->Is<Constant>());
     auto lhs = inst->Val()->As<Constant>()->value;
@@ -79,7 +79,7 @@ TEST_F(IR_InstructionTest, CreateNegation) {
         b.builder.Negation(b.builder.ir.types.Get<type::I32>(), b.builder.Constant(4_i));
 
     ASSERT_TRUE(inst->Is<Unary>());
-    EXPECT_EQ(inst->GetKind(), Unary::Kind::kNegation);
+    EXPECT_EQ(inst->kind, Unary::Kind::kNegation);
 
     ASSERT_TRUE(inst->Val()->Is<Constant>());
     auto lhs = inst->Val()->As<Constant>()->value;
@@ -92,7 +92,7 @@ TEST_F(IR_InstructionTest, Unary_Usage) {
     const auto* inst =
         b.builder.Negation(b.builder.ir.types.Get<type::I32>(), b.builder.Constant(4_i));
 
-    EXPECT_EQ(inst->GetKind(), Unary::Kind::kNegation);
+    EXPECT_EQ(inst->kind, Unary::Kind::kNegation);
 
     ASSERT_NE(inst->Val(), nullptr);
     ASSERT_EQ(inst->Val()->Usage().Length(), 1u);

src/tint/ir/value.h

@@ -47,7 +47,7 @@ class Value : public utils::Castable<Value> {
     utils::VectorRef<const Instruction*> Usage() const { return uses_; }
 
     /// @returns the type of the value
-    virtual const type::Type* Type() const = 0;
+    virtual const type::Type* Type() const { return nullptr; }
 
   protected:
     /// Constructor

src/tint/ir/var.cc

@@ -20,7 +20,7 @@ TINT_INSTANTIATE_TYPEINFO(tint::ir::Var);
 namespace tint::ir {
 
 Var::Var(const type::Type* ty, builtin::AddressSpace addr_space, builtin::Access acc)
-    : Base(ty), address_space(addr_space), access(acc) {}
+    : type(ty), address_space(addr_space), access(acc) {}
 
 Var::~Var() = default;
 

src/tint/ir/var.h

@@ -26,7 +26,7 @@ namespace tint::ir {
 class Var : public utils::Castable<Var, Instruction> {
   public:
     /// Constructor
-    /// @param type the type
+    /// @param type the type of the var
     /// @param address_space the address space of the var
     /// @param access the access mode of the var
     Var(const type::Type* type, builtin::AddressSpace address_space, builtin::Access access);
@@ -37,6 +37,12 @@ class Var : public utils::Castable<Var, Instruction> {
     Var& operator=(const Var& inst) = delete;
     Var& operator=(Var&& inst) = delete;
 
+    /// @returns the type of the var
+    const type::Type* Type() const override { return type; }
+
+    /// the result type of the instruction
+    const type::Type* type = nullptr;
+
     /// The variable address space
     builtin::AddressSpace address_space = builtin::AddressSpace::kUndefined;
 

src/tint/writer/spirv/generator_impl_ir.cc

@@ -241,14 +241,14 @@ uint32_t GeneratorImplIr::EmitBinary(const ir::Binary* binary) {
 
     // Determine the opcode.
     spv::Op op = spv::Op::Max;
-    switch (binary->GetKind()) {
+    switch (binary->kind) {
         case ir::Binary::Kind::kAdd: {
             op = binary->Type()->is_integer_scalar_or_vector() ? spv::Op::OpIAdd : spv::Op::OpFAdd;
             break;
         }
         default: {
             TINT_ICE(Writer, diagnostics_)
-                << "unimplemented binary instruction: " << static_cast<uint32_t>(binary->GetKind());
+                << "unimplemented binary instruction: " << static_cast<uint32_t>(binary->kind);
         }
     }