[clang] [llvm] [HLSL] Disallow named struct types as parameters in target extension types (PR #115971)

Joshua Batista via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 25 10:41:38 PST 2024


https://github.com/bob80905 updated https://github.com/llvm/llvm-project/pull/115971

>From 7413ceb21a6e0ac9212ef6317763ee0eff559612 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Tue, 12 Nov 2024 16:44:00 -0800
Subject: [PATCH 01/10] print struct body within target ext ty context

---
 .../test/AST/HLSL/StructuredBuffer-AST-2.hlsl | 30 +++++++++++++++++++
 llvm/lib/IR/AsmWriter.cpp                     | 10 +++++--
 2 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl

diff --git a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
new file mode 100644
index 00000000000000..73073b3f6f2839
--- /dev/null
+++ b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_dxc -T cs_6_6 %s | FileCheck %s
+
+// The purpose of this test is to ensure that the AST writer
+// only emits struct bodies when within the context of a 
+// larger object that is being outputted on the RHS.
+
+
+// note that "{ <4 x float> }" in the check below is a struct type, but only the
+// body is emitted on the RHS because we are already in the context of a
+// target extension type definition (class.hlsl::StructuredBuffer)
+// CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", { <4 x float> }, 0, 0), %struct.mystruct }
+// CHECK: %struct.mystruct = type { <4 x float> }
+// CHECK: %dx.types.Handle = type { ptr }
+// CHECK: %dx.types.ResBind = type { i32, i32, i32, i8 }
+// CHECK: %dx.types.ResourceProperties = type { i32, i32 }
+
+struct mystruct
+{
+    float4 Color;
+};
+
+StructuredBuffer<mystruct> my_buffer : register(t2, space4);
+
+export float4 test()
+{
+    return my_buffer[0].Color;
+}
+
+[numthreads(1,1,1)]
+void main() {}
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 3c1cb76622bbb7..d2705ff4b30fb8 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -649,8 +649,14 @@ void TypePrinting::print(Type *Ty, raw_ostream &OS) {
     OS << "target(\"";
     printEscapedString(Ty->getTargetExtName(), OS);
     OS << "\"";
-    for (Type *Inner : TETy->type_params())
-      OS << ", " << *Inner;
+    for (Type *Inner : TETy->type_params()) {
+      OS << ", ";
+      if (Inner->isStructTy()) {
+        StructType *STy = cast<StructType>(Inner);
+        printStructBody(STy, OS);
+      } else
+        OS << *Inner;
+    }
     for (unsigned IntParam : TETy->int_params())
       OS << ", " << IntParam;
     OS << ")";

>From 69d90f218b9702f4bd194507e54b5af86e38d3b3 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 13 Nov 2024 11:55:28 -0800
Subject: [PATCH 02/10] fix runline

---
 clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
index 73073b3f6f2839..6b71ddd567249f 100644
--- a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
+++ b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_dxc -T cs_6_6 %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-unknown-shadermodel6.6-compute -S -finclude-default-header -o - %s | FileCheck %s
 
 // The purpose of this test is to ensure that the AST writer
 // only emits struct bodies when within the context of a 

>From 3a1bdbacf556e2abffc2172721d66291e5f1e3ce Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 13 Nov 2024 13:54:54 -0800
Subject: [PATCH 03/10] undo change

---
 llvm/lib/IR/AsmWriter.cpp | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index d2705ff4b30fb8..e1c8fa834984d9 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -649,14 +649,8 @@ void TypePrinting::print(Type *Ty, raw_ostream &OS) {
     OS << "target(\"";
     printEscapedString(Ty->getTargetExtName(), OS);
     OS << "\"";
-    for (Type *Inner : TETy->type_params()) {
-      OS << ", ";
-      if (Inner->isStructTy()) {
-        StructType *STy = cast<StructType>(Inner);
-        printStructBody(STy, OS);
-      } else
-        OS << *Inner;
-    }
+    for (Type *Inner : TETy->type_params()) 
+      OS << ", " << *Inner;
     for (unsigned IntParam : TETy->int_params())
       OS << ", " << IntParam;
     OS << ")";

>From c3e6120c0d3efb7b5aeae864e164976a1d869b6f Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 21 Nov 2024 17:08:57 -0800
Subject: [PATCH 04/10] convert named to anonymous structs when creating target
 ext types

---
 clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl |  9 +++++----
 llvm/lib/IR/Type.cpp                            | 13 +++++++++++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
index 6b71ddd567249f..1edec7835f16c7 100644
--- a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
+++ b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
@@ -1,13 +1,14 @@
 // RUN: %clang_cc1 -triple dxil-unknown-shadermodel6.6-compute -S -finclude-default-header -o - %s | FileCheck %s
 
-// The purpose of this test is to ensure that the AST writer
-// only emits struct bodies when within the context of a 
-// larger object that is being outputted on the RHS.
-
+// The purpose of this test is to ensure that the target
+// extension type associated with the structured buffer only
+// contains anonymous struct types, rather than named
+// struct types
 
 // note that "{ <4 x float> }" in the check below is a struct type, but only the
 // body is emitted on the RHS because we are already in the context of a
 // target extension type definition (class.hlsl::StructuredBuffer)
+
 // CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", { <4 x float> }, 0, 0), %struct.mystruct }
 // CHECK: %struct.mystruct = type { <4 x float> }
 // CHECK: %dx.types.Handle = type { ptr }
diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index 88ede0d35fa3ee..bc285fe31c44ca 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -794,8 +794,17 @@ TargetExtType::TargetExtType(LLVMContext &C, StringRef Name,
   // Parameter storage immediately follows the class in allocation.
   Type **Params = reinterpret_cast<Type **>(this + 1);
   ContainedTys = Params;
-  for (Type *T : Types)
-    *Params++ = T;
+  for (Type *T : Types) {
+    // target ext type parameters may not be named struct types
+    // so we should convert any named struct types to anonymous
+    // struct types in the parameter list
+    Type *ConvertedTy = T;
+    if (auto *STy = dyn_cast<StructType>(T)) {
+      if (STy->hasName())
+        ConvertedTy = StructType::get(C, STy->elements(), /*isPacked=*/false);
+    }
+    *Params++ = ConvertedTy;
+  }
 
   setSubclassData(Ints.size());
   unsigned *IntParamSpace = reinterpret_cast<unsigned *>(Params);

>From 0d2579a66d3374114b0e0df4bab21d30f04a23c6 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 21 Nov 2024 17:16:43 -0800
Subject: [PATCH 05/10] clang-format

---
 llvm/lib/IR/AsmWriter.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index e1c8fa834984d9..3c1cb76622bbb7 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -649,7 +649,7 @@ void TypePrinting::print(Type *Ty, raw_ostream &OS) {
     OS << "target(\"";
     printEscapedString(Ty->getTargetExtName(), OS);
     OS << "\"";
-    for (Type *Inner : TETy->type_params()) 
+    for (Type *Inner : TETy->type_params())
       OS << ", " << *Inner;
     for (unsigned IntParam : TETy->int_params())
       OS << ", " << IntParam;

>From b24c6d7188db07f5e37d818f894b43a0e50bb9fb Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 21 Nov 2024 17:57:08 -0800
Subject: [PATCH 06/10] add assert, move conversion

---
 clang/lib/CodeGen/Targets/DirectX.cpp | 13 +++++++++++--
 llvm/lib/IR/Type.cpp                  |  8 ++++----
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/clang/lib/CodeGen/Targets/DirectX.cpp b/clang/lib/CodeGen/Targets/DirectX.cpp
index 7935f7ae370047..afa534ba2d0e47 100644
--- a/clang/lib/CodeGen/Targets/DirectX.cpp
+++ b/clang/lib/CodeGen/Targets/DirectX.cpp
@@ -43,9 +43,18 @@ llvm::Type *DirectXTargetCodeGenInfo::getHLSLType(CodeGenModule &CGM,
     if (ContainedTy.isNull())
       return nullptr;
 
-    // convert element type
     llvm::Type *ElemType = CGM.getTypes().ConvertType(ContainedTy);
 
+    // target ext type parameters may not be named struct types
+    // so we should convert any named struct types to anonymous
+    // struct types in the parameter list
+    llvm::Type *ConvertedElemType = ElemType;
+    if (auto *STy = dyn_cast<llvm::StructType>(ElemType)) {
+      if (STy->hasName())
+        ConvertedElemType =
+            llvm::StructType::get(Ctx, STy->elements(), /*isPacked=*/false);
+    }
+
     llvm::StringRef TypeName =
         ResAttrs.RawBuffer ? "dx.RawBuffer" : "dx.TypedBuffer";
     SmallVector<unsigned, 3> Ints = {/*IsWriteable*/ ResAttrs.ResourceClass ==
@@ -54,7 +63,7 @@ llvm::Type *DirectXTargetCodeGenInfo::getHLSLType(CodeGenModule &CGM,
     if (!ResAttrs.RawBuffer)
       Ints.push_back(/*IsSigned*/ ContainedTy->isSignedIntegerType());
 
-    return llvm::TargetExtType::get(Ctx, TypeName, {ElemType}, Ints);
+    return llvm::TargetExtType::get(Ctx, TypeName, {ConvertedElemType}, Ints);
   }
   case llvm::dxil::ResourceClass::CBuffer:
     llvm_unreachable("dx.CBuffer handles are not implemented yet");
diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index bc285fe31c44ca..c9795fa335795c 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -799,10 +799,10 @@ TargetExtType::TargetExtType(LLVMContext &C, StringRef Name,
     // so we should convert any named struct types to anonymous
     // struct types in the parameter list
     Type *ConvertedTy = T;
-    if (auto *STy = dyn_cast<StructType>(T)) {
-      if (STy->hasName())
-        ConvertedTy = StructType::get(C, STy->elements(), /*isPacked=*/false);
-    }
+    if (auto *STy = dyn_cast<StructType>(T))
+      assert(!STy->hasName() &&
+             "named structs are not allowed in target extension types");
+
     *Params++ = ConvertedTy;
   }
 

>From e3f9344682659b718b2fb992d7781d852c5eb50c Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 22 Nov 2024 10:21:09 -0800
Subject: [PATCH 07/10] change asmwriter

---
 clang/lib/CodeGen/Targets/DirectX.cpp           | 12 +-----------
 clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl |  4 ++--
 llvm/lib/IR/AsmWriter.cpp                       |  6 ++++--
 llvm/lib/IR/Type.cpp                            | 13 ++-----------
 4 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/clang/lib/CodeGen/Targets/DirectX.cpp b/clang/lib/CodeGen/Targets/DirectX.cpp
index afa534ba2d0e47..257cf0ee8822fd 100644
--- a/clang/lib/CodeGen/Targets/DirectX.cpp
+++ b/clang/lib/CodeGen/Targets/DirectX.cpp
@@ -45,16 +45,6 @@ llvm::Type *DirectXTargetCodeGenInfo::getHLSLType(CodeGenModule &CGM,
 
     llvm::Type *ElemType = CGM.getTypes().ConvertType(ContainedTy);
 
-    // target ext type parameters may not be named struct types
-    // so we should convert any named struct types to anonymous
-    // struct types in the parameter list
-    llvm::Type *ConvertedElemType = ElemType;
-    if (auto *STy = dyn_cast<llvm::StructType>(ElemType)) {
-      if (STy->hasName())
-        ConvertedElemType =
-            llvm::StructType::get(Ctx, STy->elements(), /*isPacked=*/false);
-    }
-
     llvm::StringRef TypeName =
         ResAttrs.RawBuffer ? "dx.RawBuffer" : "dx.TypedBuffer";
     SmallVector<unsigned, 3> Ints = {/*IsWriteable*/ ResAttrs.ResourceClass ==
@@ -63,7 +53,7 @@ llvm::Type *DirectXTargetCodeGenInfo::getHLSLType(CodeGenModule &CGM,
     if (!ResAttrs.RawBuffer)
       Ints.push_back(/*IsSigned*/ ContainedTy->isSignedIntegerType());
 
-    return llvm::TargetExtType::get(Ctx, TypeName, {ConvertedElemType}, Ints);
+    return llvm::TargetExtType::get(Ctx, TypeName, {ElemType}, Ints);
   }
   case llvm::dxil::ResourceClass::CBuffer:
     llvm_unreachable("dx.CBuffer handles are not implemented yet");
diff --git a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
index 1edec7835f16c7..a45995bc6e34ea 100644
--- a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
+++ b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-unknown-shadermodel6.6-compute -S -finclude-default-header -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-compute -S -finclude-default-header -o - %s | FileCheck %s
 
 // The purpose of this test is to ensure that the target
 // extension type associated with the structured buffer only
@@ -9,7 +9,7 @@
 // body is emitted on the RHS because we are already in the context of a
 // target extension type definition (class.hlsl::StructuredBuffer)
 
-// CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", { <4 x float> }, 0, 0), %struct.mystruct }
+// CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", %struct.mystruct, 0, 0), %struct.mystruct }
 // CHECK: %struct.mystruct = type { <4 x float> }
 // CHECK: %dx.types.Handle = type { ptr }
 // CHECK: %dx.types.ResBind = type { i32, i32, i32, i8 }
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 3c1cb76622bbb7..f8e17ac7bd6567 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -649,8 +649,10 @@ void TypePrinting::print(Type *Ty, raw_ostream &OS) {
     OS << "target(\"";
     printEscapedString(Ty->getTargetExtName(), OS);
     OS << "\"";
-    for (Type *Inner : TETy->type_params())
-      OS << ", " << *Inner;
+    for (Type *Inner : TETy->type_params()) {
+      OS << ", ";
+      Inner->print(OS, /*IsForDebug=*/false, /*NoDetails=*/true);
+    }
     for (unsigned IntParam : TETy->int_params())
       OS << ", " << IntParam;
     OS << ")";
diff --git a/llvm/lib/IR/Type.cpp b/llvm/lib/IR/Type.cpp
index c9795fa335795c..88ede0d35fa3ee 100644
--- a/llvm/lib/IR/Type.cpp
+++ b/llvm/lib/IR/Type.cpp
@@ -794,17 +794,8 @@ TargetExtType::TargetExtType(LLVMContext &C, StringRef Name,
   // Parameter storage immediately follows the class in allocation.
   Type **Params = reinterpret_cast<Type **>(this + 1);
   ContainedTys = Params;
-  for (Type *T : Types) {
-    // target ext type parameters may not be named struct types
-    // so we should convert any named struct types to anonymous
-    // struct types in the parameter list
-    Type *ConvertedTy = T;
-    if (auto *STy = dyn_cast<StructType>(T))
-      assert(!STy->hasName() &&
-             "named structs are not allowed in target extension types");
-
-    *Params++ = ConvertedTy;
-  }
+  for (Type *T : Types)
+    *Params++ = T;
 
   setSubclassData(Ints.size());
   unsigned *IntParamSpace = reinterpret_cast<unsigned *>(Params);

>From 97b07451e51a42e07e4dd63995761c93ddb14541 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Fri, 22 Nov 2024 16:58:45 -0800
Subject: [PATCH 08/10] add test to typestest.cpp

---
 clang/lib/CodeGen/Targets/DirectX.cpp         |  1 +
 .../test/AST/HLSL/StructuredBuffer-AST-2.hlsl |  5 +--
 llvm/unittests/IR/TypesTest.cpp               | 33 +++++++++++++++++++
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/clang/lib/CodeGen/Targets/DirectX.cpp b/clang/lib/CodeGen/Targets/DirectX.cpp
index 257cf0ee8822fd..7935f7ae370047 100644
--- a/clang/lib/CodeGen/Targets/DirectX.cpp
+++ b/clang/lib/CodeGen/Targets/DirectX.cpp
@@ -43,6 +43,7 @@ llvm::Type *DirectXTargetCodeGenInfo::getHLSLType(CodeGenModule &CGM,
     if (ContainedTy.isNull())
       return nullptr;
 
+    // convert element type
     llvm::Type *ElemType = CGM.getTypes().ConvertType(ContainedTy);
 
     llvm::StringRef TypeName =
diff --git a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
index a45995bc6e34ea..27d9184904ab75 100644
--- a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
+++ b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-compute -S -finclude-default-header -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-compute -emit-llvm -finclude-default-header -o - %s | FileCheck %s
 
 // The purpose of this test is to ensure that the target
 // extension type associated with the structured buffer only
@@ -11,9 +11,6 @@
 
 // CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", %struct.mystruct, 0, 0), %struct.mystruct }
 // CHECK: %struct.mystruct = type { <4 x float> }
-// CHECK: %dx.types.Handle = type { ptr }
-// CHECK: %dx.types.ResBind = type { i32, i32, i32, i8 }
-// CHECK: %dx.types.ResourceProperties = type { i32, i32 }
 
 struct mystruct
 {
diff --git a/llvm/unittests/IR/TypesTest.cpp b/llvm/unittests/IR/TypesTest.cpp
index 855262fc4e787f..a3319026fbffae 100644
--- a/llvm/unittests/IR/TypesTest.cpp
+++ b/llvm/unittests/IR/TypesTest.cpp
@@ -40,10 +40,43 @@ TEST(TypesTest, TargetExtType) {
   Type *A = TargetExtType::get(Context, "typea");
   Type *Aparam = TargetExtType::get(Context, "typea", {}, {0, 1});
   Type *Aparam2 = TargetExtType::get(Context, "typea", {}, {0, 1});
+
   // Opaque types with same parameters are identical...
   EXPECT_EQ(Aparam, Aparam2);
   // ... but just having the same name is not enough.
   EXPECT_NE(A, Aparam);
+
+  // ensure struct types in targest extension types
+  // only show the struct name, not the struct body
+  Type *Int32Type = Type::getInt32Ty(Context);
+  Type *FloatType = Type::getFloatTy(Context);
+  std::vector<Type *> OriginalElements = {Int32Type, FloatType};
+  StructType *Struct = llvm::StructType::create(Context, "MyStruct");
+  Struct->setBody(OriginalElements);
+
+  // the other struct is different only in that it's an anonymous struct,
+  // without a name
+  StructType *OtherStruct =
+      StructType::get(Context, Struct->elements(), /*isPacked=*/false);
+
+  Type *TargetExtensionType =
+      TargetExtType::get(Context, "structTET", {Struct}, {0, 1});
+  Type *OtherTargetExtensionType =
+      TargetExtType::get(Context, "structTET", {OtherStruct}, {0, 1});
+
+  SmallVector<char, 50> TETV;
+  SmallVector<char, 50> OtherTETV;
+
+  llvm::raw_svector_ostream TETStream(TETV);
+  TargetExtensionType->print(TETStream);
+
+  llvm::raw_svector_ostream OtherTETStream(OtherTETV);
+  OtherTargetExtensionType->print(OtherTETStream);
+
+  EXPECT_STREQ(TETStream.str().str().data(),
+               "target(\"structTET\", %MyStruct, 0, 1)");
+  EXPECT_STREQ(OtherTETStream.str().str().data(),
+               "target(\"structTET\", { i32, float }, 0, 1)");
 }
 
 TEST(TypedPointerType, PrintTest) {

>From c68234e14eb7119b3dc3cb8b6ce9e94f14802dae Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 25 Nov 2024 09:15:54 -0800
Subject: [PATCH 09/10] update tests

---
 .../builtins/AppendStructuredBuffer-elementtype.hlsl           | 3 ++-
 .../builtins/ConsumeStructuredBuffer-elementtype.hlsl          | 3 ++-
 .../RasterizerOrderedStructuredBuffer-elementtype.hlsl         | 3 ++-
 clang/test/CodeGenHLSL/builtins/hlsl_resource_t.hlsl           | 3 ++-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/clang/test/CodeGenHLSL/builtins/AppendStructuredBuffer-elementtype.hlsl b/clang/test/CodeGenHLSL/builtins/AppendStructuredBuffer-elementtype.hlsl
index 1e8aae588fc33d..85face8eaeb6c8 100644
--- a/clang/test/CodeGenHLSL/builtins/AppendStructuredBuffer-elementtype.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/AppendStructuredBuffer-elementtype.hlsl
@@ -18,7 +18,8 @@ struct MyStruct {
 // DXIL: %"class.hlsl::AppendStructuredBuffer.9" = type { target("dx.RawBuffer", <3 x i32>, 1, 0)
 // DXIL: %"class.hlsl::AppendStructuredBuffer.10" = type { target("dx.RawBuffer", <2 x half>, 1, 0)
 // DXIL: %"class.hlsl::AppendStructuredBuffer.11" = type { target("dx.RawBuffer", <3 x float>, 1, 0)
-// DXIL: %"class.hlsl::AppendStructuredBuffer.12" = type { target("dx.RawBuffer", %struct.MyStruct = type { <4 x float>, <2 x i32>, [8 x i8] }, 1, 0)
+// DXIL: %"class.hlsl::AppendStructuredBuffer.12" = type { target("dx.RawBuffer", %struct.MyStruct, 1, 0)
+// DXIL: %struct.MyStruct = type { <4 x float>, <2 x i32>, [8 x i8] }
 
 AppendStructuredBuffer<int16_t> BufI16;
 AppendStructuredBuffer<uint16_t> BufU16;
diff --git a/clang/test/CodeGenHLSL/builtins/ConsumeStructuredBuffer-elementtype.hlsl b/clang/test/CodeGenHLSL/builtins/ConsumeStructuredBuffer-elementtype.hlsl
index f8574c6460d4e1..5ed9e9ad8160ff 100644
--- a/clang/test/CodeGenHLSL/builtins/ConsumeStructuredBuffer-elementtype.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/ConsumeStructuredBuffer-elementtype.hlsl
@@ -18,7 +18,8 @@ struct MyStruct {
 // DXIL: %"class.hlsl::ConsumeStructuredBuffer.9" = type { target("dx.RawBuffer", <3 x i32>, 1, 0)
 // DXIL: %"class.hlsl::ConsumeStructuredBuffer.10" = type { target("dx.RawBuffer", <2 x half>, 1, 0)
 // DXIL: %"class.hlsl::ConsumeStructuredBuffer.11" = type { target("dx.RawBuffer", <3 x float>, 1, 0)
-// DXIL: %"class.hlsl::ConsumeStructuredBuffer.12" = type { target("dx.RawBuffer", %struct.MyStruct = type { <4 x float>, <2 x i32>, [8 x i8] }, 1, 0)
+// DXIL: %"class.hlsl::ConsumeStructuredBuffer.12" = type { target("dx.RawBuffer", %struct.MyStruct, 1, 0)
+// DXIL: %struct.MyStruct = type { <4 x float>, <2 x i32>, [8 x i8] }
 
 ConsumeStructuredBuffer<int16_t> BufI16;
 ConsumeStructuredBuffer<uint16_t> BufU16;
diff --git a/clang/test/CodeGenHLSL/builtins/RasterizerOrderedStructuredBuffer-elementtype.hlsl b/clang/test/CodeGenHLSL/builtins/RasterizerOrderedStructuredBuffer-elementtype.hlsl
index 1de3a4e3961af8..53a75a4561df5c 100644
--- a/clang/test/CodeGenHLSL/builtins/RasterizerOrderedStructuredBuffer-elementtype.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/RasterizerOrderedStructuredBuffer-elementtype.hlsl
@@ -22,7 +22,8 @@ struct MyStruct {
 // DXIL: %"class.hlsl::RasterizerOrderedStructuredBuffer.11" = type { target("dx.RawBuffer", <3 x i32>, 1, 1)
 // DXIL: %"class.hlsl::RasterizerOrderedStructuredBuffer.12" = type { target("dx.RawBuffer", <2 x half>, 1, 1)
 // DXIL: %"class.hlsl::RasterizerOrderedStructuredBuffer.13" = type { target("dx.RawBuffer", <3 x float>, 1, 1)
-// DXIL: %"class.hlsl::RasterizerOrderedStructuredBuffer.14" = type { target("dx.RawBuffer", %struct.MyStruct = type { <4 x float>, <2 x i32>, [8 x i8] }, 1, 1)
+// DXIL: %"class.hlsl::RasterizerOrderedStructuredBuffer.14" = type { target("dx.RawBuffer", %struct.MyStruct, 1, 1)
+// DXIL: %struct.MyStruct = type { <4 x float>, <2 x i32>, [8 x i8] }
 
 RasterizerOrderedStructuredBuffer<int16_t> BufI16;
 RasterizerOrderedStructuredBuffer<uint16_t> BufU16;
diff --git a/clang/test/CodeGenHLSL/builtins/hlsl_resource_t.hlsl b/clang/test/CodeGenHLSL/builtins/hlsl_resource_t.hlsl
index c206874a4ca94a..62fc46c284c310 100644
--- a/clang/test/CodeGenHLSL/builtins/hlsl_resource_t.hlsl
+++ b/clang/test/CodeGenHLSL/builtins/hlsl_resource_t.hlsl
@@ -3,7 +3,8 @@
 using handle_float_t = __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::contained_type(float)]];
 
 // CHECK: %"class.hlsl::RWBuffer" = type { target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
-// CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", %struct.MyStruct = type { <4 x float>, <2 x i32>, [8 x i8] }, 0, 0)
+// CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", %struct.MyStruct, 0, 0)
+// CHECK: %struct.MyStruct = type { <4 x float>, <2 x i32>, [8 x i8] }
 
 // CHECK: define void @_Z2faU9_Res_u_CTfu17__hlsl_resource_t(target("dx.TypedBuffer", float, 1, 0, 0) %a)
 // CHECK: call void @_Z4foo1U9_Res_u_CTfu17__hlsl_resource_t(target("dx.TypedBuffer", float, 1, 0, 0) %0)

>From e4c9bf53d99447429cdf437fabf897d3656d9487 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 25 Nov 2024 10:41:19 -0800
Subject: [PATCH 10/10] address Justin

---
 .../test/AST/HLSL/StructuredBuffer-AST-2.hlsl | 28 ------------------
 llvm/unittests/IR/TypesTest.cpp               | 29 +++++++++----------
 2 files changed, 14 insertions(+), 43 deletions(-)
 delete mode 100644 clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl

diff --git a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl b/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
deleted file mode 100644
index 27d9184904ab75..00000000000000
--- a/clang/test/AST/HLSL/StructuredBuffer-AST-2.hlsl
+++ /dev/null
@@ -1,28 +0,0 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-compute -emit-llvm -finclude-default-header -o - %s | FileCheck %s
-
-// The purpose of this test is to ensure that the target
-// extension type associated with the structured buffer only
-// contains anonymous struct types, rather than named
-// struct types
-
-// note that "{ <4 x float> }" in the check below is a struct type, but only the
-// body is emitted on the RHS because we are already in the context of a
-// target extension type definition (class.hlsl::StructuredBuffer)
-
-// CHECK: %"class.hlsl::StructuredBuffer" = type { target("dx.RawBuffer", %struct.mystruct, 0, 0), %struct.mystruct }
-// CHECK: %struct.mystruct = type { <4 x float> }
-
-struct mystruct
-{
-    float4 Color;
-};
-
-StructuredBuffer<mystruct> my_buffer : register(t2, space4);
-
-export float4 test()
-{
-    return my_buffer[0].Color;
-}
-
-[numthreads(1,1,1)]
-void main() {}
diff --git a/llvm/unittests/IR/TypesTest.cpp b/llvm/unittests/IR/TypesTest.cpp
index a3319026fbffae..62db5660220f87 100644
--- a/llvm/unittests/IR/TypesTest.cpp
+++ b/llvm/unittests/IR/TypesTest.cpp
@@ -50,31 +50,30 @@ TEST(TypesTest, TargetExtType) {
   // only show the struct name, not the struct body
   Type *Int32Type = Type::getInt32Ty(Context);
   Type *FloatType = Type::getFloatTy(Context);
-  std::vector<Type *> OriginalElements = {Int32Type, FloatType};
-  StructType *Struct = llvm::StructType::create(Context, "MyStruct");
-  Struct->setBody(OriginalElements);
+  std::array<Type *, 2> Elements = {Int32Type, FloatType};
 
-  // the other struct is different only in that it's an anonymous struct,
-  // without a name
+  StructType *Struct = llvm::StructType::create(Context, Elements, "MyStruct",
+                                                /*isPacked=*/false);
+  SmallVector<char, 50> TETV;
+  llvm::raw_svector_ostream TETStream(TETV);
+  Type *TargetExtensionType =
+      TargetExtType::get(Context, "structTET", {Struct}, {0, 1});
+  TargetExtensionType->print(TETStream);
+
+  EXPECT_STREQ(TETStream.str().str().data(),
+               "target(\"structTET\", %MyStruct, 0, 1)");
+
+  // ensure that literal structs in the target extension type print the struct
+  // body
   StructType *OtherStruct =
       StructType::get(Context, Struct->elements(), /*isPacked=*/false);
 
-  Type *TargetExtensionType =
-      TargetExtType::get(Context, "structTET", {Struct}, {0, 1});
   Type *OtherTargetExtensionType =
       TargetExtType::get(Context, "structTET", {OtherStruct}, {0, 1});
-
-  SmallVector<char, 50> TETV;
   SmallVector<char, 50> OtherTETV;
-
-  llvm::raw_svector_ostream TETStream(TETV);
-  TargetExtensionType->print(TETStream);
-
   llvm::raw_svector_ostream OtherTETStream(OtherTETV);
   OtherTargetExtensionType->print(OtherTETStream);
 
-  EXPECT_STREQ(TETStream.str().str().data(),
-               "target(\"structTET\", %MyStruct, 0, 1)");
   EXPECT_STREQ(OtherTETStream.str().str().data(),
                "target(\"structTET\", { i32, float }, 0, 1)");
 }



More information about the cfe-commits mailing list