spirv-fuzz: Refactor variable creation (#3414)

Fixes #3413.

source/fuzz/fuzzer_pass_push_ids_through_variables.cpp

@@ -104,13 +104,37 @@ void FuzzerPassPushIdsThroughVariables::Apply() {
         // If the pointer type does not exist, then create it.
         FindOrCreatePointerType(basic_type_id, variable_storage_class);
 
+        // TODO(https://github.com/KhronosGroup/SPIRV-Tools/issues/3403):
+        //  type support here is limited by the FindOrCreateZeroConstant
+        //  function.
+        const auto* type_inst =
+            GetIRContext()->get_def_use_mgr()->GetDef(basic_type_id);
+        assert(type_inst);
+        switch (type_inst->opcode()) {
+          case SpvOpTypeBool:
+          case SpvOpTypeFloat:
+          case SpvOpTypeInt:
+          case SpvOpTypeArray:
+          case SpvOpTypeMatrix:
+          case SpvOpTypeVector:
+          case SpvOpTypeStruct:
+            break;
+          default:
+            return;
+        }
+
+        // Create a constant to initialize the variable from. This might update
+        // module's id bound so it must be done before any fresh ids are
+        // computed.
+        auto initializer_id = FindOrCreateZeroConstant(basic_type_id);
+
         // Applies the push id through variable transformation.
         ApplyTransformation(TransformationPushIdThroughVariable(
             value_instructions[GetFuzzerContext()->RandomIndex(
                                    value_instructions)]
                 ->result_id(),
             GetFuzzerContext()->GetFreshId(), GetFuzzerContext()->GetFreshId(),
-            variable_storage_class, instruction_descriptor));
+            variable_storage_class, initializer_id, instruction_descriptor));
       });
 }
 

source/fuzz/fuzzer_pass_push_ids_through_variables.h

@@ -30,7 +30,7 @@ class FuzzerPassPushIdsThroughVariables : public FuzzerPass {
       FuzzerContext* fuzzer_context,
       protobufs::TransformationSequence* transformations);
 
-  ~FuzzerPassPushIdsThroughVariables();
+  ~FuzzerPassPushIdsThroughVariables() override;
 
   void Apply() override;
 };

source/fuzz/fuzzer_util.cpp

@@ -583,6 +583,75 @@ void AddVariableIdToEntryPointInterfaces(opt::IRContext* context, uint32_t id) {
   }
 }
 
+void AddGlobalVariable(opt::IRContext* context, uint32_t result_id,
+                       uint32_t type_id, SpvStorageClass storage_class,
+                       uint32_t initializer_id) {
+  // Check various preconditions.
+  assert((storage_class == SpvStorageClassPrivate ||
+          storage_class == SpvStorageClassWorkgroup) &&
+         "Variable's storage class must be either Private or Workgroup");
+
+  auto* type_inst = context->get_def_use_mgr()->GetDef(type_id);
+  (void)type_inst;  // Variable becomes unused in release mode.
+  assert(type_inst && type_inst->opcode() == SpvOpTypePointer &&
+         GetStorageClassFromPointerType(type_inst) == storage_class &&
+         "Variable's type is invalid");
+
+  if (storage_class == SpvStorageClassWorkgroup) {
+    assert(initializer_id == 0);
+  }
+
+  if (initializer_id != 0) {
+    const auto* constant_inst =
+        context->get_def_use_mgr()->GetDef(initializer_id);
+    (void)constant_inst;  // Variable becomes unused in release mode.
+    assert(constant_inst && spvOpcodeIsConstant(constant_inst->opcode()) &&
+           GetPointeeTypeIdFromPointerType(type_inst) ==
+               constant_inst->type_id() &&
+           "Initializer is invalid");
+  }
+
+  opt::Instruction::OperandList operands = {
+      {SPV_OPERAND_TYPE_STORAGE_CLASS, {static_cast<uint32_t>(storage_class)}}};
+
+  if (initializer_id) {
+    operands.push_back({SPV_OPERAND_TYPE_ID, {initializer_id}});
+  }
+
+  context->module()->AddGlobalValue(MakeUnique<opt::Instruction>(
+      context, SpvOpVariable, type_id, result_id, std::move(operands)));
+
+  AddVariableIdToEntryPointInterfaces(context, result_id);
+}
+
+void AddLocalVariable(opt::IRContext* context, uint32_t result_id,
+                      uint32_t type_id, uint32_t function_id,
+                      uint32_t initializer_id) {
+  // Check various preconditions.
+  auto* type_inst = context->get_def_use_mgr()->GetDef(type_id);
+  (void)type_inst;  // Variable becomes unused in release mode.
+  assert(type_inst && type_inst->opcode() == SpvOpTypePointer &&
+         GetStorageClassFromPointerType(type_inst) == SpvStorageClassFunction &&
+         "Variable's type is invalid");
+
+  const auto* constant_inst =
+      context->get_def_use_mgr()->GetDef(initializer_id);
+  (void)constant_inst;  // Variable becomes unused in release mode.
+  assert(constant_inst && spvOpcodeIsConstant(constant_inst->opcode()) &&
+         GetPointeeTypeIdFromPointerType(type_inst) ==
+             constant_inst->type_id() &&
+         "Initializer is invalid");
+
+  auto* function = FindFunction(context, function_id);
+  assert(function && "Function id is invalid");
+
+  function->begin()->begin()->InsertBefore(MakeUnique<opt::Instruction>(
+      context, SpvOpVariable, type_id, result_id,
+      opt::Instruction::OperandList{
+          {SPV_OPERAND_TYPE_STORAGE_CLASS, {SpvStorageClassFunction}},
+          {SPV_OPERAND_TYPE_ID, {initializer_id}}}));
+}
+
 }  // namespace fuzzerutil
 
 }  // namespace fuzz

source/fuzz/fuzzer_util.h

@@ -226,6 +226,32 @@ bool GlobalVariablesMustBeDeclaredInEntryPointInterfaces(
 // from an entry point function, to be listed in that function's interface.
 void AddVariableIdToEntryPointInterfaces(opt::IRContext* context, uint32_t id);
 
+// Adds a global variable with storage class |storage_class| to the module, with
+// type |type_id| and either no initializer or |initializer_id| as an
+// initializer, depending on whether |initializer_id| is 0. The global variable
+// has result id |result_id|.
+//
+// - |type_id| must be the id of a pointer type with the same storage class as
+//   |storage_class|.
+// - |storage_class| must be Private or Workgroup.
+// - |initializer_id| must be 0 if |storage_class| is Workgroup, and otherwise
+//   may either be 0 or the id of a constant whose type is the pointee type of
+//   |type_id|.
+void AddGlobalVariable(opt::IRContext* context, uint32_t result_id,
+                       uint32_t type_id, SpvStorageClass storage_class,
+                       uint32_t initializer_id);
+
+// Adds an instruction to the start of |function_id|, of the form:
+//   |result_id| = OpVariable |type_id| Function |initializer_id|.
+//
+// - |type_id| must be the id of a pointer type with Function storage class.
+// - |initializer_id| must be the id of a constant with the same type as the
+//   pointer's pointee type.
+// - |function_id| must be the id of a function.
+void AddLocalVariable(opt::IRContext* context, uint32_t result_id,
+                      uint32_t type_id, uint32_t function_id,
+                      uint32_t initializer_id);
+
 }  // namespace fuzzerutil
 
 }  // namespace fuzz

source/fuzz/protobufs/spvtoolsfuzz.proto

@@ -1020,12 +1020,15 @@ message TransformationPushIdThroughVariable {
   // A fresh id for the variable to be stored to.
   uint32 variable_id = 3;
 
+  // Constant to initialize the variable from.
+  uint32 initializer_id = 4;
+
   // The variable storage class (global or local).
-  uint32 variable_storage_class = 4;
+  uint32 variable_storage_class = 5;
 
   // A descriptor for an instruction which the new OpStore
   // and OpLoad instructions might be inserted before.
-  InstructionDescriptor instruction_descriptor = 5;
+  InstructionDescriptor instruction_descriptor = 6;
 
 }
 

source/fuzz/transformation_add_global_variable.cpp

@@ -93,20 +93,12 @@ bool TransformationAddGlobalVariable::IsApplicable(
 void TransformationAddGlobalVariable::Apply(
     opt::IRContext* ir_context,
     TransformationContext* transformation_context) const {
-  opt::Instruction::OperandList input_operands;
-  input_operands.push_back(
-      {SPV_OPERAND_TYPE_STORAGE_CLASS, {message_.storage_class()}});
-  if (message_.initializer_id()) {
-    input_operands.push_back(
-        {SPV_OPERAND_TYPE_ID, {message_.initializer_id()}});
-  }
-  ir_context->module()->AddGlobalValue(MakeUnique<opt::Instruction>(
-      ir_context, SpvOpVariable, message_.type_id(), message_.fresh_id(),
-      input_operands));
-  fuzzerutil::UpdateModuleIdBound(ir_context, message_.fresh_id());
+  fuzzerutil::AddGlobalVariable(
+      ir_context, message_.fresh_id(), message_.type_id(),
+      static_cast<SpvStorageClass>(message_.storage_class()),
+      message_.initializer_id());
 
-  fuzzerutil::AddVariableIdToEntryPointInterfaces(ir_context,
-                                                  message_.fresh_id());
+  fuzzerutil::UpdateModuleIdBound(ir_context, message_.fresh_id());
 
   if (message_.value_is_irrelevant()) {
     transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant(

source/fuzz/transformation_add_local_variable.cpp

@@ -70,18 +70,12 @@ bool TransformationAddLocalVariable::IsApplicable(
 void TransformationAddLocalVariable::Apply(
     opt::IRContext* ir_context,
     TransformationContext* transformation_context) const {
-  fuzzerutil::UpdateModuleIdBound(ir_context, message_.fresh_id());
-  fuzzerutil::FindFunction(ir_context, message_.function_id())
-      ->begin()
-      ->begin()
-      ->InsertBefore(MakeUnique<opt::Instruction>(
-          ir_context, SpvOpVariable, message_.type_id(), message_.fresh_id(),
-          opt::Instruction::OperandList(
-              {{SPV_OPERAND_TYPE_STORAGE_CLASS,
-                {
+  fuzzerutil::AddLocalVariable(ir_context, message_.fresh_id(),
+                               message_.type_id(), message_.function_id(),
+                               message_.initializer_id());
+
+  fuzzerutil::UpdateModuleIdBound(ir_context, message_.fresh_id());
 
-                    SpvStorageClassFunction}},
-               {SPV_OPERAND_TYPE_ID, {message_.initializer_id()}}})));
   if (message_.value_is_irrelevant()) {
     transformation_context->GetFactManager()->AddFactValueOfPointeeIsIrrelevant(
         message_.fresh_id());

source/fuzz/transformation_push_id_through_variable.cpp

@@ -27,12 +27,13 @@ TransformationPushIdThroughVariable::TransformationPushIdThroughVariable(
 
 TransformationPushIdThroughVariable::TransformationPushIdThroughVariable(
     uint32_t value_id, uint32_t value_synonym_id, uint32_t variable_id,
-    uint32_t variable_storage_class,
+    uint32_t variable_storage_class, uint32_t initializer_id,
     const protobufs::InstructionDescriptor& instruction_descriptor) {
   message_.set_value_id(value_id);
   message_.set_value_synonym_id(value_synonym_id);
   message_.set_variable_id(variable_id);
   message_.set_variable_storage_class(variable_storage_class);
+  message_.set_initializer_id(initializer_id);
   *message_.mutable_instruction_descriptor() = instruction_descriptor;
 }
 
@@ -85,6 +86,14 @@ bool TransformationPushIdThroughVariable::IsApplicable(
           message_.variable_storage_class() == SpvStorageClassFunction) &&
          "The variable storage class must be private or function.");
 
+  // Check that initializer is valid.
+  const auto* constant_inst =
+      ir_context->get_def_use_mgr()->GetDef(message_.initializer_id());
+  if (!constant_inst || !spvOpcodeIsConstant(constant_inst->opcode()) ||
+      value_instruction->type_id() != constant_inst->type_id()) {
+    return false;
+  }
+
   // |message_.value_id| must be available at the insertion point.
   return fuzzerutil::IdIsAvailableBeforeInstruction(
       ir_context, instruction_to_insert_before, message_.value_id());
@@ -103,28 +112,23 @@ void TransformationPushIdThroughVariable::Apply(
   assert(pointer_type_id && "The required pointer type must be available.");
 
   // Adds whether a global or local variable.
-  fuzzerutil::UpdateModuleIdBound(ir_context, message_.variable_id());
   if (message_.variable_storage_class() == SpvStorageClassPrivate) {
-    ir_context->module()->AddGlobalValue(MakeUnique<opt::Instruction>(
-        ir_context, SpvOpVariable, pointer_type_id, message_.variable_id(),
-        opt::Instruction::OperandList(
-            {{SPV_OPERAND_TYPE_STORAGE_CLASS, {SpvStorageClassPrivate}}})));
-
-    fuzzerutil::AddVariableIdToEntryPointInterfaces(ir_context,
-                                                    message_.variable_id());
+    fuzzerutil::AddGlobalVariable(ir_context, message_.variable_id(),
+                                  pointer_type_id, SpvStorageClassPrivate,
+                                  message_.initializer_id());
   } else {
-    ir_context
-        ->get_instr_block(
-            FindInstruction(message_.instruction_descriptor(), ir_context))
-        ->GetParent()
-        ->begin()
-        ->begin()
-        ->InsertBefore(MakeUnique<opt::Instruction>(
-            ir_context, SpvOpVariable, pointer_type_id, message_.variable_id(),
-            opt::Instruction::OperandList({{SPV_OPERAND_TYPE_STORAGE_CLASS,
-                                            {SpvStorageClassFunction}}})));
+    auto function_id = ir_context
+                           ->get_instr_block(FindInstruction(
+                               message_.instruction_descriptor(), ir_context))
+                           ->GetParent()
+                           ->result_id();
+    fuzzerutil::AddLocalVariable(ir_context, message_.variable_id(),
+                                 pointer_type_id, function_id,
+                                 message_.initializer_id());
   }
 
+  fuzzerutil::UpdateModuleIdBound(ir_context, message_.variable_id());
+
   // Stores value id to variable id.
   FindInstruction(message_.instruction_descriptor(), ir_context)
       ->InsertBefore(MakeUnique<opt::Instruction>(

source/fuzz/transformation_push_id_through_variable.h

@@ -31,6 +31,7 @@ class TransformationPushIdThroughVariable : public Transformation {
   TransformationPushIdThroughVariable(
       uint32_t value_id, uint32_t value_synonym_fresh_id,
       uint32_t variable_fresh_id, uint32_t variable_storage_class,
+      uint32_t initializer_id,
       const protobufs::InstructionDescriptor& instruction_descriptor);
 
   // - |message_.value_id| must be an instruction result id that has the same
@@ -39,6 +40,9 @@ class TransformationPushIdThroughVariable : public Transformation {
   // - |message_.variable_id| must be fresh
   // - |message_.variable_storage_class| must be either StorageClassPrivate or
   //   StorageClassFunction
+  // - |message_.initializer_id| must be a result id of some constant in the
+  //   module. Its type must be equal to the pointee type of the variable that
+  //   will be created.
   // - |message_.instruction_descriptor| must identify an instruction
   //   which it is valid to insert the OpStore and OpLoad instructions before it
   //   and must be belongs to a reachable block.

test/fuzz/transformation_push_id_through_variable_test.cpp

@@ -107,12 +107,13 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   uint32_t value_id = 21;
   uint32_t value_synonym_id = 62;
   uint32_t variable_id = 63;
+  uint32_t initializer_id = 23;
   uint32_t variable_storage_class = SpvStorageClassPrivate;
   auto instruction_descriptor =
       MakeInstructionDescriptor(95, SpvOpReturnValue, 0);
   auto transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   ASSERT_TRUE(
       transformation.IsApplicable(context.get(), transformation_context));
 
@@ -120,11 +121,12 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   value_id = 80;
   value_synonym_id = 60;
   variable_id = 61;
+  initializer_id = 80;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(38, SpvOpAccessChain, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   ASSERT_FALSE(
       transformation.IsApplicable(context.get(), transformation_context));
 
@@ -132,11 +134,12 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   value_id = 80;
   value_synonym_id = 62;
   variable_id = 63;
+  initializer_id = 80;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(64, SpvOpAccessChain, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   ASSERT_FALSE(
       transformation.IsApplicable(context.get(), transformation_context));
 
@@ -145,11 +148,12 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   value_id = 24;
   value_synonym_id = 62;
   variable_id = 63;
+  initializer_id = 24;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(27, SpvOpVariable, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   ASSERT_FALSE(
       transformation.IsApplicable(context.get(), transformation_context));
 
@@ -157,11 +161,12 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   value_id = 80;
   value_synonym_id = 62;
   variable_id = 63;
+  initializer_id = 80;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(100, SpvOpUnreachable, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   ASSERT_FALSE(
       transformation.IsApplicable(context.get(), transformation_context));
 
@@ -169,11 +174,12 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   value_id = 64;
   value_synonym_id = 62;
   variable_id = 63;
+  initializer_id = 23;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(95, SpvOpReturnValue, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   ASSERT_FALSE(
       transformation.IsApplicable(context.get(), transformation_context));
 
@@ -181,11 +187,12 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   value_id = 80;
   value_synonym_id = 62;
   variable_id = 63;
+  initializer_id = 80;
   variable_storage_class = SpvStorageClassPrivate;
   instruction_descriptor = MakeInstructionDescriptor(95, SpvOpReturnValue, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   ASSERT_FALSE(
       transformation.IsApplicable(context.get(), transformation_context));
 
@@ -193,11 +200,12 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   value_id = 93;
   value_synonym_id = 62;
   variable_id = 63;
+  initializer_id = 93;
   variable_storage_class = SpvStorageClassInput;
   instruction_descriptor = MakeInstructionDescriptor(95, SpvOpReturnValue, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
 #ifndef NDEBUG
   ASSERT_DEATH(
       transformation.IsApplicable(context.get(), transformation_context),
@@ -208,11 +216,38 @@ TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
   value_id = 95;
   value_synonym_id = 62;
   variable_id = 63;
+  initializer_id = 80;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(40, SpvOpAccessChain, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
+  ASSERT_FALSE(
+      transformation.IsApplicable(context.get(), transformation_context));
+
+  // Variable initializer is not constant.
+  value_id = 95;
+  value_synonym_id = 62;
+  variable_id = 63;
+  initializer_id = 95;
+  variable_storage_class = SpvStorageClassFunction;
+  instruction_descriptor = MakeInstructionDescriptor(40, SpvOpAccessChain, 0);
+  transformation = TransformationPushIdThroughVariable(
+      value_id, value_synonym_id, variable_id, variable_storage_class,
+      initializer_id, instruction_descriptor);
+  ASSERT_FALSE(
+      transformation.IsApplicable(context.get(), transformation_context));
+
+  // Variable initializer has wrong type.
+  value_id = 95;
+  value_synonym_id = 62;
+  variable_id = 63;
+  initializer_id = 93;
+  variable_storage_class = SpvStorageClassFunction;
+  instruction_descriptor = MakeInstructionDescriptor(40, SpvOpAccessChain, 0);
+  transformation = TransformationPushIdThroughVariable(
+      value_id, value_synonym_id, variable_id, variable_storage_class,
+      initializer_id, instruction_descriptor);
   ASSERT_FALSE(
       transformation.IsApplicable(context.get(), transformation_context));
 }
@@ -298,52 +333,57 @@ TEST(TransformationPushIdThroughVariableTest, Apply) {
   uint32_t value_id = 80;
   uint32_t value_synonym_id = 100;
   uint32_t variable_id = 101;
+  uint32_t initializer_id = 80;
   uint32_t variable_storage_class = SpvStorageClassFunction;
   auto instruction_descriptor =
       MakeInstructionDescriptor(38, SpvOpAccessChain, 0);
   auto transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   transformation.Apply(context.get(), &transformation_context);
 
   value_id = 21;
   value_synonym_id = 102;
   variable_id = 103;
+  initializer_id = 21;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(38, SpvOpAccessChain, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   transformation.Apply(context.get(), &transformation_context);
 
   value_id = 95;
   value_synonym_id = 104;
   variable_id = 105;
+  initializer_id = 80;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(95, SpvOpReturnValue, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   transformation.Apply(context.get(), &transformation_context);
 
   value_id = 80;
   value_synonym_id = 106;
   variable_id = 107;
+  initializer_id = 80;
   variable_storage_class = SpvStorageClassFunction;
   instruction_descriptor = MakeInstructionDescriptor(95, SpvOpReturnValue, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   transformation.Apply(context.get(), &transformation_context);
 
   value_id = 21;
   value_synonym_id = 108;
   variable_id = 109;
+  initializer_id = 21;
   variable_storage_class = SpvStorageClassPrivate;
   instruction_descriptor = MakeInstructionDescriptor(95, SpvOpReturnValue, 0);
   transformation = TransformationPushIdThroughVariable(
       value_id, value_synonym_id, variable_id, variable_storage_class,
-      instruction_descriptor);
+      initializer_id, instruction_descriptor);
   transformation.Apply(context.get(), &transformation_context);
 
   std::string variant_shader = R"(
@@ -380,11 +420,11 @@ TEST(TransformationPushIdThroughVariableTest, Apply) {
          %91 = OpTypePointer Input %90
          %92 = OpVariable %91 Input
          %93 = OpConstantComposite %90 %24 %24 %24 %24
-        %109 = OpVariable %51 Private
+        %109 = OpVariable %51 Private %21
           %4 = OpFunction %2 None %3
           %5 = OpLabel
-        %103 = OpVariable %15 Function
-        %101 = OpVariable %9 Function
+        %103 = OpVariable %15 Function %21
+        %101 = OpVariable %9 Function %80
          %20 = OpVariable %9 Function
          %27 = OpVariable %9 Function
          %22 = OpAccessChain %15 %20 %14
@@ -413,8 +453,8 @@ TEST(TransformationPushIdThroughVariableTest, Apply) {
          %12 = OpFunction %6 None %10
          %11 = OpFunctionParameter %9
          %13 = OpLabel
-        %107 = OpVariable %9 Function
-        %105 = OpVariable %9 Function
+        %107 = OpVariable %9 Function %80
+        %105 = OpVariable %9 Function %80
          %46 = OpCopyObject %9 %11
          %16 = OpAccessChain %15 %11 %14
          %95 = OpCopyObject %8 %80