Revert DxilGenerationPass changes and remove scalar store tests

Reverts the DxilGenerationPass ByteAddressBuffer scalar store fix from
PR microsoft#8274 — it was out of scope and incomplete (sext fallback is wrong
for min16uint). Scalar ByteAddressBuffer template store widening should
be handled during CodeGen where signedness info is still available.

Removes scalar load/store tests from min_precision_raw_load_store.hlsl
since scalar ByteAddressBuffer::Store<min16int>() hits a pre-existing
crash in TranslateMinPrecisionRawBuffer (cast<StructType> on non-struct
resource type). Test now covers vector loads/stores only, which is the
scope of the original fix.

Co-authored-by: Copilot <[email protected]>

Author: alsepkow

lib/HLSL/DxilGenerationPass.cpp

@@ -993,10 +993,11 @@ void ReplaceMinPrecisionRawBufferStoreByType(
         Args.emplace_back(NewV);
       }
     } else if (FromTy->isIntegerTy()) {
-      // Since we are extending integer, we have to know if we should sign ext
-      // or zero ext. For StructuredBuffers we get signedness from the struct
-      // type annotation. For ByteAddressBuffer (raw buffers) there is no struct
-      // annotation, so we fall back to sext as a conservative default.
+      // This case only applies to typed buffer since Store operation of byte
+      // address buffer for min precision is handled by implicit conversion on
+      // intrinsic call. Since we are extending integer, we have to know if we
+      // should sign ext or zero ext. We can do this by iterating checking the
+      // size of the element at struct type and comp type at type annotation
       CallInst *handleCI = dyn_cast<CallInst>(
           CI->getArgOperand(DxilInst_RawBufferStore::arg_uav));
       DXASSERT(handleCI,
@@ -1006,50 +1007,34 @@ void ReplaceMinPrecisionRawBufferStoreByType(
                "otherwise fail to handle for buffer store lost its retTy");
       StructType *STy = dyn_cast<StructType>(resTyIt->second);
 
-      StructType *InnerSTy =
-          STy ? dyn_cast<StructType>(STy->getElementType(0)) : nullptr;
-      DxilStructAnnotation *SAnnot =
-          InnerSTy ? typeSys.GetStructAnnotation(InnerSTy) : nullptr;
-
-      if (SAnnot) {
-        // StructuredBuffer path: use struct annotation to determine signedness.
-        ConstantInt *offsetInt = dyn_cast<ConstantInt>(
-            CI->getArgOperand(DxilInst_RawBufferStore::arg_elementOffset));
-        unsigned offset = offsetInt->getSExtValue();
-        unsigned currentOffset = 0;
-        for (DxilStructTypeIterator iter = begin(InnerSTy, SAnnot),
-                                    ItEnd = end(InnerSTy, SAnnot);
-             iter != ItEnd; ++iter) {
-          std::pair<Type *, DxilFieldAnnotation *> pair = *iter;
-          currentOffset += DL.getTypeAllocSize(pair.first);
-          if (currentOffset > offset) {
-            if (pair.second->GetCompType().IsUIntTy()) {
-              for (unsigned i = 4; i < 8; ++i) {
-                Value *NewV = CIBuilder.CreateZExt(CI->getArgOperand(i), ToTy);
-                Args.emplace_back(NewV);
-              }
-              break;
-            } else if (pair.second->GetCompType().IsIntTy()) {
-              for (unsigned i = 4; i < 8; ++i) {
-                Value *NewV = CIBuilder.CreateSExt(CI->getArgOperand(i), ToTy);
-                Args.emplace_back(NewV);
-              }
-              break;
-            } else {
-              DXASSERT(false, "Invalid comp type");
+      STy = cast<StructType>(STy->getElementType(0));
+      DxilStructAnnotation *SAnnot = typeSys.GetStructAnnotation(STy);
+      ConstantInt *offsetInt = dyn_cast<ConstantInt>(
+          CI->getArgOperand(DxilInst_RawBufferStore::arg_elementOffset));
+      unsigned offset = offsetInt->getSExtValue();
+      unsigned currentOffset = 0;
+      for (DxilStructTypeIterator iter = begin(STy, SAnnot),
+                                  ItEnd = end(STy, SAnnot);
+           iter != ItEnd; ++iter) {
+        std::pair<Type *, DxilFieldAnnotation *> pair = *iter;
+        currentOffset += DL.getTypeAllocSize(pair.first);
+        if (currentOffset > offset) {
+          if (pair.second->GetCompType().IsUIntTy()) {
+            for (unsigned i = 4; i < 8; ++i) {
+              Value *NewV = CIBuilder.CreateZExt(CI->getArgOperand(i), ToTy);
+              Args.emplace_back(NewV);
             }
+            break;
+          } else if (pair.second->GetCompType().IsIntTy()) {
+            for (unsigned i = 4; i < 8; ++i) {
+              Value *NewV = CIBuilder.CreateSExt(CI->getArgOperand(i), ToTy);
+              Args.emplace_back(NewV);
+            }
+            break;
+          } else {
+            DXASSERT(false, "Invalid comp type");
           }
         }
-      } else {
-        // ByteAddressBuffer path: no struct annotation available, so
-        // signedness is unknown. Default to sext.
-        for (unsigned i = 4; i < 8; ++i) {
-          Value *Arg = CI->getArgOperand(i);
-          if (isa<UndefValue>(Arg))
-            Args.emplace_back(UndefValue::get(ToTy));
-          else
-            Args.emplace_back(CIBuilder.CreateSExt(Arg, ToTy));
-        }
       }
     }
 

tools/clang/test/HLSLFileCheck/hlsl/objects/ByteAddressBuffer/min_precision_raw_load_store.hlsl

@@ -30,33 +30,9 @@ void main() {
   // CHECK: call void @dx.op.rawBufferVectorStore.v3f32
   g_buf.Store< min16float3 >(60, vf);
 
-  // === Scalar loads/stores (RawBufferLoad/Store) ===
-
-  // min16int scalar: should use i32 rawBufferStore
-  // CHECK: call %dx.types.ResRet.i32 @dx.op.rawBufferLoad.i32
-  min16int si = g_buf.Load< min16int >(72);
-  // CHECK: call void @dx.op.rawBufferStore.i32
-  g_buf.Store< min16int >(76, si);
-
-  // min16uint scalar: should use i32 rawBufferStore
-  // CHECK: call %dx.types.ResRet.i32 @dx.op.rawBufferLoad.i32
-  min16uint su = g_buf.Load< min16uint >(80);
-  // CHECK: call void @dx.op.rawBufferStore.i32
-  g_buf.Store< min16uint >(84, su);
-
-  // min16float scalar: should use f32 rawBufferStore
-  // CHECK: call %dx.types.ResRet.f32 @dx.op.rawBufferLoad.f32
-  min16float sf = g_buf.Load< min16float >(88);
-  // CHECK: call void @dx.op.rawBufferStore.f32
-  g_buf.Store< min16float >(92, sf);
-
-  // Verify i16/f16 ops are NOT used.
+  // Verify i16/f16 ops are NOT used for vector loads/stores.
   // CHECK-NOT: rawBufferVectorLoad.v{{[0-9]+}}i16
   // CHECK-NOT: rawBufferVectorStore.v{{[0-9]+}}i16
   // CHECK-NOT: rawBufferVectorLoad.v{{[0-9]+}}f16
   // CHECK-NOT: rawBufferVectorStore.v{{[0-9]+}}f16
-  // CHECK-NOT: rawBufferLoad.i16
-  // CHECK-NOT: rawBufferStore.i16
-  // CHECK-NOT: rawBufferLoad.f16
-  // CHECK-NOT: rawBufferStore.f16
 }