Set the resource kind as u32 for boolean resources

It was producing a validation error previously because they are represented as u32 and i1 is not valid for output resources. This preserves the cmp conversions for values read from the resource, but satisfies the validation requirement. Performs some incidental consolidation of types that are represented as u32. Related to microsoft#7079 remove dead array handing code The array type checks were for a type extracted from a vector. Arrays cannot be elements of vectors. This is leftover from when that was not the case. The code shows as never executed in coverage
pow2clk · Feb 13, 2025 · 7d337af · 7d337af
1 parent c2f85a6
commit 7d337af
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 90 deletions.
diff --git a/lib/DXIL/DxilResource.cpp b/lib/DXIL/DxilResource.cpp
@@ -30,18 +30,7 @@ DxilResource::DxilResource()
 
 CompType DxilResource::GetCompType() const { return m_CompType; }
 
-void DxilResource::SetCompType(const CompType CT) {
-  // Translate packed types to u32
-  switch (CT.GetKind()) {
-  case CompType::Kind::PackedS8x32:
-  case CompType::Kind::PackedU8x32:
-    m_CompType = CompType::getU32();
-    break;
-  default:
-    m_CompType = CT;
-    break;
-  }
-}
+void DxilResource::SetCompType(const CompType CT) { m_CompType = CT; }
 
 Type *DxilResource::GetRetType() const {
   Type *Ty = GetHLSLType()->getPointerElementType();

diff --git a/lib/HLSL/HLOperationLower.cpp b/lib/HLSL/HLOperationLower.cpp
@@ -8436,89 +8436,52 @@ void TranslateStructBufSubscriptUser(Instruction *user, Value *handle,
                                    : stInst->getValueOperand()->getType();
     Type *pOverloadTy = Ty->getScalarType();
     Value *offset = baseOffset;
-    unsigned arraySize = 1;
-    Value *eltSize = nullptr;
 
-    if (pOverloadTy->isArrayTy()) {
-      arraySize = pOverloadTy->getArrayNumElements();
-      eltSize = OP->GetU32Const(
-          DL.getTypeAllocSize(pOverloadTy->getArrayElementType()));
+    if (ldInst) {
+      unsigned numComponents = 0;
+      Value *newLd = nullptr;
+      if (VectorType *VTy = dyn_cast<VectorType>(Ty))
+        numComponents = VTy->getNumElements();
+      else
+        numComponents = 1;
 
-      pOverloadTy = pOverloadTy->getArrayElementType()->getScalarType();
-    }
+      if (ResKind == HLResource::Kind::TypedBuffer) {
+        // Typed buffer cannot have offsets, they must be loaded all at once
+        ResRetValueArray ResRet = GenerateTypedBufferLoad(
+            handle, pOverloadTy, bufIdx, status, OP, Builder);
 
-    if (ldInst) {
-      auto LdElement = [=](Value *offset, IRBuilder<> &Builder) -> Value * {
-        unsigned numComponents = 0;
-        if (VectorType *VTy = dyn_cast<VectorType>(Ty)) {
-          numComponents = VTy->getNumElements();
-        } else {
-          numComponents = 1;
-        }
+        newLd = ExtractFromTypedBufferLoad(ResRet, Ty, offset, Builder);
+      } else {
+        Value *ResultElts[4];
         Constant *alignment =
             OP->GetI32Const(DL.getTypeAllocSize(Ty->getScalarType()));
-        if (ResKind == HLResource::Kind::TypedBuffer) {
-          // Typed buffer cannot have offsets, they must be loaded all at once
-          ResRetValueArray ResRet = GenerateTypedBufferLoad(
-              handle, pOverloadTy, bufIdx, status, OP, Builder);
-
-          return ExtractFromTypedBufferLoad(ResRet, Ty, offset, Builder);
-        } else {
-          Value *ResultElts[4];
-          GenerateRawBufLd(handle, bufIdx, offset, status, pOverloadTy,
-                           ResultElts, OP, Builder, numComponents, alignment);
-          return ScalarizeElements(Ty, ResultElts, Builder);
-        }
-      };
-
-      Value *newLd = LdElement(offset, Builder);
-      if (arraySize > 1) {
-        newLd =
-            Builder.CreateInsertValue(UndefValue::get(Ty), newLd, (uint64_t)0);
-
-        for (unsigned i = 1; i < arraySize; i++) {
-          offset = Builder.CreateAdd(offset, eltSize);
-          Value *eltLd = LdElement(offset, Builder);
-          newLd = Builder.CreateInsertValue(newLd, eltLd, i);
-        }
+        GenerateRawBufLd(handle, bufIdx, offset, status, pOverloadTy,
+                         ResultElts, OP, Builder, numComponents, alignment);
+        newLd = ScalarizeElements(Ty, ResultElts, Builder);
       }
+
       ldInst->replaceAllUsesWith(newLd);
     } else {
       Value *val = stInst->getValueOperand();
-      auto StElement = [&](Value *offset, Value *val, IRBuilder<> &Builder) {
-        Value *undefVal = llvm::UndefValue::get(pOverloadTy);
-        Value *vals[] = {undefVal, undefVal, undefVal, undefVal};
-        uint8_t mask = 0;
-        if (Ty->isVectorTy()) {
-          unsigned vectorNumElements = Ty->getVectorNumElements();
-          DXASSERT(vectorNumElements <= 4, "up to 4 elements in vector");
-          assert(vectorNumElements <= 4);
-          for (unsigned i = 0; i < vectorNumElements; i++) {
-            vals[i] = Builder.CreateExtractElement(val, i);
-            mask |= (1 << i);
-          }
-        } else {
-          vals[0] = val;
-          mask = DXIL::kCompMask_X;
-        }
-        Constant *alignment =
-            OP->GetI32Const(DL.getTypeAllocSize(Ty->getScalarType()));
-        GenerateStructBufSt(handle, bufIdx, offset, pOverloadTy, OP, Builder,
-                            vals, mask, alignment);
-      };
-      if (arraySize > 1)
-        val = Builder.CreateExtractValue(val, 0);
-
-      StElement(offset, val, Builder);
-      if (arraySize > 1) {
-        val = stInst->getValueOperand();
-
-        for (unsigned i = 1; i < arraySize; i++) {
-          offset = Builder.CreateAdd(offset, eltSize);
-          Value *eltVal = Builder.CreateExtractValue(val, i);
-          StElement(offset, eltVal, Builder);
+      Value *undefVal = llvm::UndefValue::get(pOverloadTy);
+      Value *vals[] = {undefVal, undefVal, undefVal, undefVal};
+      uint8_t mask = 0;
+      if (Ty->isVectorTy()) {
+        unsigned vectorNumElements = Ty->getVectorNumElements();
+        DXASSERT(vectorNumElements <= 4, "up to 4 elements in vector");
+        assert(vectorNumElements <= 4);
+        for (unsigned i = 0; i < vectorNumElements; i++) {
+          vals[i] = Builder.CreateExtractElement(val, i);
+          mask |= (1 << i);
         }
+      } else {
+        vals[0] = val;
+        mask = DXIL::kCompMask_X;
       }
+      Constant *alignment =
+          OP->GetI32Const(DL.getTypeAllocSize(Ty->getScalarType()));
+      GenerateStructBufSt(handle, bufIdx, offset, pOverloadTy, OP, Builder,
+                          vals, mask, alignment);
     }
     user->eraseFromParent();
   } else if (BitCastInst *BCI = dyn_cast<BitCastInst>(user)) {

diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp
@@ -3539,11 +3539,20 @@ bool CGMSHLSLRuntime::SetUAVSRV(SourceLocation loc,
     if (const BuiltinType *BTy = EltTy->getAs<BuiltinType>()) {
       CompType::Kind kind = BuiltinTyToCompTy(BTy, bHasNormAttribute && bSNorm,
                                               bHasNormAttribute && !bSNorm);
-      // 64bits types are implemented with u32.
-      if (kind == CompType::Kind::U64 || kind == CompType::Kind::I64 ||
-          kind == CompType::Kind::SNormF64 ||
-          kind == CompType::Kind::UNormF64 || kind == CompType::Kind::F64) {
+      // Boolean, 64-bit, and packed types are implemented with u32.
+      switch (kind) {
+      case CompType::Kind::I1:
+      case CompType::Kind::U64:
+      case CompType::Kind::I64:
+      case CompType::Kind::F64:
+      case CompType::Kind::SNormF64:
+      case CompType::Kind::UNormF64:
+      case CompType::Kind::PackedS8x32:
+      case CompType::Kind::PackedU8x32:
         kind = CompType::Kind::U32;
+        break;
+      default:
+        break;
       }
       hlslRes->SetCompType(kind);
     } else {

diff --git a/tools/clang/test/CodeGenDXIL/hlsl/intrinsics/buffer-load-stores.hlsl b/tools/clang/test/CodeGenDXIL/hlsl/intrinsics/buffer-load-stores.hlsl
@@ -1,10 +1,10 @@
 // RUN: %dxc -DTYPE=float4    -DIX=SIx -T vs_6_6 %s | FileCheck %s
-// RUiN: %dxc -DTYPE=bool4     -DIX=SIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,I1
+// RUN: %dxc -DTYPE=bool4     -DIX=SIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,I1
 // RUN: %dxc -DTYPE=uint64_t2 -DIX=SIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,I64
 // RUN: %dxc -DTYPE=double2   -DIX=SIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,F64
 
 // RUN: %dxc -DTYPE=float4    -DIX=VIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,VIX
-// RUiN: %dxc -DTYPE=bool4     -DIX=VIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,VIX,I1
+// RUN: %dxc -DTYPE=bool4     -DIX=VIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,VIX,I1
 // RUN: %dxc -DTYPE=uint64_t2 -DIX=VIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,VIX,I64
 // RUN: %dxc -DTYPE=double2   -DIX=VIx -T vs_6_6 %s | FileCheck %s --check-prefixes=CHECK,VIX,F64
 

diff --git a/tools/clang/test/CodeGenHLSL/dot.hlsl → ...test/CodeGenDXIL/hlsl/intrinsics/dot.hlsl b/tools/clang/test/CodeGenHLSL/dot.hlsl → ...test/CodeGenDXIL/hlsl/intrinsics/dot.hlsl
diff --git a/tools/clang/test/CodeGenHLSL/vector-and.hlsl → ...deGenDXIL/hlsl/intrinsics/vector-and.hlsl b/tools/clang/test/CodeGenHLSL/vector-and.hlsl → ...deGenDXIL/hlsl/intrinsics/vector-and.hlsl
diff --git a/tools/clang/test/CodeGenHLSL/vector-or.hlsl → ...odeGenDXIL/hlsl/intrinsics/vector-or.hlsl b/tools/clang/test/CodeGenHLSL/vector-or.hlsl → ...odeGenDXIL/hlsl/intrinsics/vector-or.hlsl