Minimal opt support for SPV_KHR_untyped_pointers (KhronosGroup#6087)

* Added support for global OpUntypedVariableKHR in IRLoader
* Added support for untyped pointers as Pointer types using a null
  element
  * Updates to type manager
* No specific pass allowances yet, but untyped pointers no longer break
  the whole pipeline

Author: alan-baker

source/opt/ir_loader.cpp

@@ -207,6 +207,7 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) {
       } else if (IsTypeInst(opcode)) {
         module_->AddType(std::move(spv_inst));
       } else if (IsConstantInst(opcode) || opcode == spv::Op::OpVariable ||
+                 opcode == spv::Op::OpUntypedVariableKHR ||
                  opcode == spv::Op::OpUndef) {
         module_->AddGlobalValue(std::move(spv_inst));
       } else if (spvIsExtendedInstruction(opcode) &&

source/opt/type_manager.cpp

@@ -374,16 +374,21 @@ uint32_t TypeManager::GetTypeInstruction(const Type* type) {
     }
     case Type::kPointer: {
       const Pointer* pointer = type->AsPointer();
-      uint32_t subtype = GetTypeInstruction(pointer->pointee_type());
-      if (subtype == 0) {
-        return 0;
+      if (pointer->is_untyped()) {
+        typeInst = MakeUnique<Instruction>(
+            context(), spv::Op::OpTypeUntypedPointerKHR, 0, id,
+            std::initializer_list<Operand>{
+                {SPV_OPERAND_TYPE_STORAGE_CLASS,
+                 {static_cast<uint32_t>(pointer->storage_class())}}});
+      } else {
+        uint32_t subtype = GetTypeInstruction(pointer->pointee_type());
+        typeInst = MakeUnique<Instruction>(
+            context(), spv::Op::OpTypePointer, 0, id,
+            std::initializer_list<Operand>{
+                {SPV_OPERAND_TYPE_STORAGE_CLASS,
+                 {static_cast<uint32_t>(pointer->storage_class())}},
+                {SPV_OPERAND_TYPE_ID, {subtype}}});
       }
-      typeInst = MakeUnique<Instruction>(
-          context(), spv::Op::OpTypePointer, 0, id,
-          std::initializer_list<Operand>{
-              {SPV_OPERAND_TYPE_STORAGE_CLASS,
-               {static_cast<uint32_t>(pointer->storage_class())}},
-              {SPV_OPERAND_TYPE_ID, {subtype}}});
       break;
     }
     case Type::kFunction: {
@@ -680,9 +685,13 @@ Type* TypeManager::RebuildType(uint32_t type_id, const Type& type) {
     }
     case Type::kPointer: {
       const Pointer* pointer_ty = type.AsPointer();
-      const Type* ele_ty = pointer_ty->pointee_type();
-      rebuilt_ty = MakeUnique<Pointer>(RebuildType(GetId(ele_ty), *ele_ty),
-                                       pointer_ty->storage_class());
+      if (pointer_ty->pointee_type()) {
+        const Type* ele_ty = pointer_ty->pointee_type();
+        rebuilt_ty = MakeUnique<Pointer>(RebuildType(GetId(ele_ty), *ele_ty),
+                                         pointer_ty->storage_class());
+      } else {
+        rebuilt_ty = MakeUnique<Pointer>(nullptr, pointer_ty->storage_class());
+      }
       break;
     }
     case Type::kFunction: {
@@ -928,6 +937,11 @@ Type* TypeManager::RecordIfTypeDefinition(const Instruction& inst) {
       id_to_incomplete_type_.erase(inst.result_id());
 
     } break;
+    case spv::Op::OpTypeUntypedPointerKHR: {
+      type = new Pointer(nullptr, static_cast<spv::StorageClass>(
+                                      inst.GetSingleWordInOperand(0)));
+      id_to_incomplete_type_.erase(inst.result_id());
+    } break;
     case spv::Op::OpTypeFunction: {
       bool incomplete_type = false;
       uint32_t return_type_id = inst.GetSingleWordInOperand(0);

source/opt/type_manager.h

@@ -34,7 +34,7 @@ namespace analysis {
 
 // Hashing functor.
 //
-// All type pointers must be non-null.
+// All type pointers must be non-null to reach here.
 struct HashTypePointer {
   size_t operator()(const Type* type) const {
     assert(type);

source/opt/types.cpp

@@ -621,24 +621,39 @@ bool Pointer::IsSameImpl(const Type* that, IsSameCache* seen) const {
   if (!p.second) {
     return true;
   }
-  bool same_pointee = pointee_type_->IsSameImpl(pt->pointee_type_, seen);
-  seen->erase(p.first);
-  if (!same_pointee) {
-    return false;
+  if (pointee_type_ != nullptr && pt->pointee_type_ != nullptr) {
+    bool same_pointee = pointee_type_->IsSameImpl(pt->pointee_type_, seen);
+    seen->erase(p.first);
+    if (!same_pointee) {
+      return false;
+    }
+  } else {
+    seen->erase(p.first);
+    // Either both are untyped or it is mixed typed and untyped.
+    if (pointee_type_ != pt->pointee_type_) {
+      return false;
+    }
   }
   return HasSameDecorations(that);
 }
 
 std::string Pointer::str() const {
   std::ostringstream os;
-  os << pointee_type_->str() << " " << static_cast<uint32_t>(storage_class_)
-     << "*";
+  if (pointee_type_) {
+    os << pointee_type_->str();
+  } else {
+    os << "untyped_ptr";
+  }
+  os << " " << static_cast<uint32_t>(storage_class_) << "*";
   return os.str();
 }
 
 size_t Pointer::ComputeExtraStateHash(size_t hash, SeenTypes* seen) const {
   hash = hash_combine(hash, uint32_t(storage_class_));
-  return pointee_type_->ComputeHashValue(hash, seen);
+  if (pointee_type_) {
+    hash = pointee_type_->ComputeHashValue(hash, seen);
+  }
+  return hash;
 }
 
 void Pointer::SetPointeeType(const Type* type) { pointee_type_ = type; }

source/opt/types.h

@@ -550,6 +550,8 @@ class Pointer : public Type {
   const Type* pointee_type() const { return pointee_type_; }
   spv::StorageClass storage_class() const { return storage_class_; }
 
+  bool is_untyped() const { return pointee_type_ == nullptr; }
+
   Pointer* AsPointer() override { return this; }
   const Pointer* AsPointer() const override { return this; }
 

test/opt/type_manager_test.cpp

@@ -149,6 +149,7 @@ std::vector<std::unique_ptr<Type>> GenerateAllTypes() {
   types.emplace_back(new Pointer(f32, spv::StorageClass::Input));
   types.emplace_back(new Pointer(sts32f32, spv::StorageClass::Function));
   types.emplace_back(new Pointer(a42f32, spv::StorageClass::Function));
+  types.emplace_back(new Pointer(nullptr, spv::StorageClass::Uniform));
 
   // Function
   types.emplace_back(new Function(voidt, {}));
@@ -249,6 +250,7 @@ TEST(TypeManager, TypeStrings) {
     %cm   = OpTypeCooperativeMatrixNV %f64 %id4 %id4 %id4
     %id2    = OpConstant %u32 2
     %cmkhr  = OpTypeCooperativeMatrixKHR %f64 %id4 %id4 %id4 %id2
+    %untyped = OpTypeUntypedPointerKHR Uniform
   )";
 
   std::vector<std::pair<uint32_t, std::string>> type_id_strs = {
@@ -288,10 +290,11 @@ TEST(TypeManager, TypeStrings) {
       {38, "[sint32, id(34), words(2,34)]"},
       {39, "<float64, 6, 6, 6>"},
       {41, "<float64, 6, 6, 6, 40>"},
+      {42, "untyped_ptr 2*"},  // Include storage class number
   };
 
   std::unique_ptr<IRContext> context =
-      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text);
+      BuildModule(SPV_ENV_UNIVERSAL_1_4, nullptr, text);
   ASSERT_NE(nullptr, context.get());  // It assembled
   TypeManager manager(nullptr, context.get());
 

test/opt/types_test.cpp

@@ -226,6 +226,7 @@ std::vector<std::unique_ptr<Type>> GenerateAllTypes() {
   types.emplace_back(new Pointer(sts32f32, spv::StorageClass::Function));
   types.emplace_back(new Pointer(a42f32, spv::StorageClass::Function));
   types.emplace_back(new Pointer(voidt, spv::StorageClass::Function));
+  types.emplace_back(new Pointer(nullptr, spv::StorageClass::Uniform));
 
   // Function
   types.emplace_back(new Function(voidt, {}));
@@ -457,6 +458,15 @@ TEST(Types, RemoveDecorations) {
   }
 }
 
+TEST(Types, UntypedPointer) {
+  std::unique_ptr<Type> type(new Pointer(nullptr, spv::StorageClass::Uniform));
+  const auto untyped = type->AsPointer();
+  EXPECT_NE(untyped, nullptr);
+  EXPECT_TRUE(untyped->is_untyped());
+  EXPECT_EQ(untyped->pointee_type(), nullptr);
+  EXPECT_EQ(untyped->storage_class(), spv::StorageClass::Uniform);
+}
+
 }  // namespace
 }  // namespace analysis
 }  // namespace opt