[PIX] Fix render-state visualization passes

lib/DxilPIXPasses/DxilOutputColorBecomesConstant.cpp

@@ -12,6 +12,7 @@
 
 #include "dxc/DXIL/DxilModule.h"
 #include "dxc/DXIL/DxilOperations.h"
+#include "dxc/DXIL/DxilTypeSystem.h"
 #include "dxc/DxilPIXPasses/DxilPIXPasses.h"
 #include "dxc/HLSL/DxilGenerationPass.h"
 #include "dxc/HLSL/DxilSpanAllocator.h"
@@ -108,52 +109,78 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) {
 
   const hlsl::DxilSignature &OutputSignature = DM.GetOutputSignature();
 
-  Function *FloatOutputFunction =
-      HlslOP->GetOpFunc(DXIL::OpCode::StoreOutput, Type::getFloatTy(Ctx));
-  Function *IntOutputFunction =
-      HlslOP->GetOpFunc(DXIL::OpCode::StoreOutput, Type::getInt32Ty(Ctx));
+  // dx.op.storeOutput has four legal overloads: f16, f32, i16 and i32.
+  // A min16float or min16int SV_Target lowers through the f16 or i16 form,
+  // as does a native half or int16_t target under -enable-16bit-types.
+  const std::array<llvm::Type *, 4> OverloadTypes{
+      Type::getHalfTy(Ctx), Type::getFloatTy(Ctx), Type::getInt16Ty(Ctx),
+      Type::getInt32Ty(Ctx)};
 
-  bool hasFloatOutputs = false;
-  bool hasIntOutputs = false;
+  std::array<Function *, 4> OutputFunctions{};
+  size_t ActiveOverload = OverloadTypes.size();
 
-  visitOutputInstructionCallers(
-      FloatOutputFunction, OutputSignature, HlslOP,
-      [&hasFloatOutputs](CallInst *) { hasFloatOutputs = true; });
+  for (size_t OverloadIndex = 0; OverloadIndex < OverloadTypes.size();
+       ++OverloadIndex) {
+    OutputFunctions[OverloadIndex] = HlslOP->GetOpFunc(
+        DXIL::OpCode::StoreOutput, OverloadTypes[OverloadIndex]);
 
-  visitOutputInstructionCallers(
-      IntOutputFunction, OutputSignature, HlslOP,
-      [&hasIntOutputs](CallInst *) { hasIntOutputs = true; });
+    bool HasTargetZeroStores = false;
+    visitOutputInstructionCallers(
+        OutputFunctions[OverloadIndex], OutputSignature, HlslOP,
+        [&HasTargetZeroStores](CallInst *) { HasTargetZeroStores = true; });
+
+    if (HasTargetZeroStores) {
+      // visitOutputInstructionCallers filters on SemanticKind::Target with
+      // GetSemanticStartIndex() == 0, so at most one overload writes
+      // SV_Target0.
+      DXASSERT(ActiveOverload == OverloadTypes.size(),
+               "Only one storeOutput overload can write SV_Target0");
+      ActiveOverload = OverloadIndex;
+    }
+  }
+
+  // GetOpFunc materialises each overload declaration on demand. Any
+  // overload with no callers must be erased before the pass returns; the
+  // validator rejects a module carrying an unused dx.op declaration.
+  struct EraseUnusedOutputFunctionsOnExit {
+    hlsl::DxilModule &DM;
+    std::array<Function *, 4> &OutputFunctions;
+    ~EraseUnusedOutputFunctionsOnExit() {
+      for (Function *OutputFunction : OutputFunctions) {
+        PIXPassHelpers::EraseIfUnused(DM, OutputFunction);
+      }
+    }
+  } EraseUnusedOutputFunctions{DM, OutputFunctions};
 
-  if (!hasFloatOutputs && !hasIntOutputs) {
-    PIXPassHelpers::EraseIfUnused(DM, FloatOutputFunction);
-    PIXPassHelpers::EraseIfUnused(DM, IntOutputFunction);
+  if (ActiveOverload == OverloadTypes.size()) {
     return false;
   }
 
-  // Otherwise, we assume the shader outputs only one or the other (because the
-  // 0th RTV can't have a mixed type)
-  DXASSERT(!hasFloatOutputs || !hasIntOutputs,
-           "Only one or the other type of output: float or int");
+  // Replacement values must match the store's own overload type.
+  llvm::Type *const OutputValueType = OverloadTypes[ActiveOverload];
+  const bool IsFloatOutput = OutputValueType->isFloatingPointTy();
 
   std::array<llvm::Value *, 4> ReplacementColors;
 
   switch (Mode) {
   case FromLiteralConstant: {
-    if (hasFloatOutputs) {
-      ReplacementColors[0] = HlslOP->GetFloatConst(Red);
-      ReplacementColors[1] = HlslOP->GetFloatConst(Green);
-      ReplacementColors[2] = HlslOP->GetFloatConst(Blue);
-      ReplacementColors[3] = HlslOP->GetFloatConst(Alpha);
-    }
-    if (hasIntOutputs) {
-      ReplacementColors[0] = HlslOP->GetI32Const(static_cast<int>(Red));
-      ReplacementColors[1] = HlslOP->GetI32Const(static_cast<int>(Green));
-      ReplacementColors[2] = HlslOP->GetI32Const(static_cast<int>(Blue));
-      ReplacementColors[3] = HlslOP->GetI32Const(static_cast<int>(Alpha));
+    const std::array<float, 4> Channels{Red, Green, Blue, Alpha};
+    for (size_t ChannelIndex = 0; ChannelIndex < Channels.size();
+         ++ChannelIndex) {
+      ReplacementColors[ChannelIndex] =
+          IsFloatOutput
+              ? ConstantFP::get(OutputValueType, Channels[ChannelIndex])
+              : ConstantInt::get(OutputValueType,
+                                 static_cast<uint64_t>(static_cast<int64_t>(
+                                     Channels[ChannelIndex])),
+                                 /*isSigned*/ true);
     }
   } break;
   case FromConstantBuffer: {
 
+    // A float4 constant buffer row is 16 bytes wide.
+    constexpr unsigned int ConstantColorCBufferSizeInBytes = 4 * sizeof(float);
+
     // Setup a constant buffer with a single float4 in it:
     SmallVector<llvm::Type *, 4> Elements{
         Type::getFloatTy(Ctx), Type::getFloatTy(Ctx), Type::getFloatTy(Ctx),
@@ -162,13 +189,31 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) {
         llvm::StructType::create(Elements, "PIX_ConstantColorCB_Type");
     std::unique_ptr<DxilCBuffer> pCBuf = llvm::make_unique<DxilCBuffer>();
     pCBuf->SetGlobalName("PIX_ConstantColorCBName");
-    pCBuf->SetGlobalSymbol(UndefValue::get(CBStructTy));
+    // The global symbol and HLSL type must be pointers to the struct so
+    // ValidateCBuffer can reach the annotation.
+    pCBuf->SetGlobalSymbol(UndefValue::get(CBStructTy->getPointerTo()));
+    pCBuf->SetHLSLType(CBStructTy->getPointerTo());
     pCBuf->SetID(static_cast<unsigned int>(DM.GetCBuffers().size()));
     pCBuf->SetSpaceID(
         (unsigned int)-2); // This is the reserved-for-tools register space
     pCBuf->SetLowerBound(0);
     pCBuf->SetRangeSize(1);
-    pCBuf->SetSize(4);
+    pCBuf->SetSize(ConstantColorCBufferSizeInBytes);
+
+    auto *StructAnnotation = DM.GetTypeSystem().GetStructAnnotation(CBStructTy);
+    if (StructAnnotation == nullptr) {
+      StructAnnotation = DM.GetTypeSystem().AddStructAnnotation(CBStructTy);
+      StructAnnotation->SetCBufferSize(ConstantColorCBufferSizeInBytes);
+      static const char *const ComponentNames[] = {"r", "g", "b", "a"};
+      for (unsigned int ComponentIndex = 0; ComponentIndex < 4;
+           ++ComponentIndex) {
+        auto &FieldAnnotation =
+            StructAnnotation->GetFieldAnnotation(ComponentIndex);
+        FieldAnnotation.SetCBufferOffset(ComponentIndex * sizeof(float));
+        FieldAnnotation.SetCompType(hlsl::DXIL::ComponentType::F32);
+        FieldAnnotation.SetFieldName(ComponentNames[ComponentIndex]);
+      }
+    }
 
     Instruction *entryPointInstruction =
         &*(PIXPassHelpers::GetEntryFunction(DM)->begin()->begin());
@@ -188,9 +233,12 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) {
 #define PIX_CONSTANT_VALUE "PIX_Constant_Color_Value"
 
     // Insert the Buffer load instruction:
-    Function *CBLoad = HlslOP->GetOpFunc(
-        OP::OpCode::CBufferLoadLegacy,
-        hasFloatOutputs ? Type::getFloatTy(Ctx) : Type::getInt32Ty(Ctx));
+    // The tools constant buffer is always four 32-bit components; PIX
+    // uploads that layout.
+    llvm::Type *const CBufferComponentType =
+        IsFloatOutput ? Type::getFloatTy(Ctx) : Type::getInt32Ty(Ctx);
+    Function *CBLoad =
+        HlslOP->GetOpFunc(OP::OpCode::CBufferLoadLegacy, CBufferComponentType);
     Constant *OpArg =
         HlslOP->GetU32Const((unsigned)OP::OpCode::CBufferLoadLegacy);
     Value *ResourceHandle = callCreateHandle;
@@ -207,54 +255,47 @@ bool DxilOutputColorBecomesConstant::runOnModule(Module &M) {
         Builder.CreateExtractValue(loadLegacy, 2, PIX_CONSTANT_VALUE "2");
     ReplacementColors[3] =
         Builder.CreateExtractValue(loadLegacy, 3, PIX_CONSTANT_VALUE "3");
+
+    // Narrow the loaded components to a 16-bit output overload.
+    if (OutputValueType != CBufferComponentType) {
+      static const char *const NarrowedNames[] = {
+          PIX_CONSTANT_VALUE "Narrowed0", PIX_CONSTANT_VALUE "Narrowed1",
+          PIX_CONSTANT_VALUE "Narrowed2", PIX_CONSTANT_VALUE "Narrowed3"};
+      for (size_t ChannelIndex = 0; ChannelIndex < ReplacementColors.size();
+           ++ChannelIndex) {
+        ReplacementColors[ChannelIndex] =
+            IsFloatOutput
+                ? Builder.CreateFPTrunc(ReplacementColors[ChannelIndex],
+                                        OutputValueType,
+                                        NarrowedNames[ChannelIndex])
+                : Builder.CreateTrunc(ReplacementColors[ChannelIndex],
+                                      OutputValueType,
+                                      NarrowedNames[ChannelIndex]);
+      }
+    }
   } break;
   default:
     assert(false);
-    return 0;
+    return false;
   }
 
   bool Modified = false;
 
-  // The StoreOutput function can store either a float or an integer, depending
-  // on the intended output render-target resource view.
-  if (hasFloatOutputs) {
-    visitOutputInstructionCallers(
-        FloatOutputFunction, OutputSignature, HlslOP,
-        [&ReplacementColors, &Modified](CallInst *CallInstruction) {
-          Modified = true;
-          // The output column is the channel (red, green, blue or alpha) within
-          // the output pixel
-          Value *OutputColumnOperand = CallInstruction->getOperand(
-              hlsl::DXIL::OperandIndex::kStoreOutputColOpIdx);
-          ConstantInt *OutputColumnConstant =
-              cast<ConstantInt>(OutputColumnOperand);
-          APInt OutputColumn = OutputColumnConstant->getValue();
-          CallInstruction->setOperand(
-              hlsl::DXIL::OperandIndex::kStoreOutputValOpIdx,
-              ReplacementColors[*OutputColumn.getRawData()]);
-        });
-  }
-
-  if (hasIntOutputs) {
-    visitOutputInstructionCallers(
-        IntOutputFunction, OutputSignature, HlslOP,
-        [&ReplacementColors, &Modified](CallInst *CallInstruction) {
-          Modified = true;
-          // The output column is the channel (red, green, blue or alpha) within
-          // the output pixel
-          Value *OutputColumnOperand = CallInstruction->getOperand(
-              hlsl::DXIL::OperandIndex::kStoreOutputColOpIdx);
-          ConstantInt *OutputColumnConstant =
-              cast<ConstantInt>(OutputColumnOperand);
-          APInt OutputColumn = OutputColumnConstant->getValue();
-          CallInstruction->setOperand(
-              hlsl::DXIL::OperandIndex::kStoreOutputValOpIdx,
-              ReplacementColors[*OutputColumn.getRawData()]);
-        });
-  }
-
-  PIXPassHelpers::EraseIfUnused(DM, FloatOutputFunction);
-  PIXPassHelpers::EraseIfUnused(DM, IntOutputFunction);
+  visitOutputInstructionCallers(
+      OutputFunctions[ActiveOverload], OutputSignature, HlslOP,
+      [&ReplacementColors, &Modified](CallInst *CallInstruction) {
+        Modified = true;
+        // The output column is the channel (red, green, blue or alpha) within
+        // the output pixel
+        Value *OutputColumnOperand = CallInstruction->getOperand(
+            hlsl::DXIL::OperandIndex::kStoreOutputColOpIdx);
+        ConstantInt *OutputColumnConstant =
+            cast<ConstantInt>(OutputColumnOperand);
+        APInt OutputColumn = OutputColumnConstant->getValue();
+        CallInstruction->setOperand(
+            hlsl::DXIL::OperandIndex::kStoreOutputValOpIdx,
+            ReplacementColors[*OutputColumn.getRawData()]);
+      });
 
   return Modified;
 }

lib/DxilPIXPasses/DxilReduceMSAAToSingleSample.cpp

@@ -13,6 +13,7 @@
 
 #include "dxc/DXIL/DxilInstructions.h"
 #include "dxc/DXIL/DxilModule.h"
+#include "dxc/DXIL/DxilResourceProperties.h"
 #include "dxc/DxilPIXPasses/DxilPIXPasses.h"
 #include "dxc/HLSL/DxilGenerationPass.h"
 
@@ -34,50 +35,72 @@ class DxilReduceMSAAToSingleSample : public ModulePass {
   bool runOnModule(Module &M) override;
 };
 
-bool DxilReduceMSAAToSingleSample::runOnModule(Module &M) {
-  DxilModule &DM = M.GetOrCreateDxilModule();
+static bool IsMultisampledSRVHandle(Value *TextureHandle, DxilModule &DM) {
+  auto *TextureHandleInst = dyn_cast<CallInst>(TextureHandle);
+  if (!TextureHandleInst)
+    return false;
+
+  if (OP::IsDxilOpFuncCallInst(TextureHandleInst, OP::OpCode::CreateHandle)) {
+    DxilInst_CreateHandle CreateHandle(TextureHandleInst);
+    if (!isa<ConstantInt>(CreateHandle.get_rangeId()))
+      return false;
+
+    if (static_cast<DXIL::ResourceClass>(
+            CreateHandle.get_resourceClass_val()) != DXIL::ResourceClass::SRV)
+      return false;
+
+    unsigned RangeId =
+        cast<ConstantInt>(CreateHandle.get_rangeId())->getLimitedValue();
+    auto Resource = DM.GetSRV(RangeId);
+    return Resource.GetKind() == DXIL::ResourceKind::Texture2DMS ||
+           Resource.GetKind() == DXIL::ResourceKind::Texture2DMSArray;
+  }
 
-  LLVMContext &Ctx = M.getContext();
-  OP *HlslOP = DM.GetOP();
+  // SM 6.6 handles carry the resource kind in the annotateHandle
+  // properties operand.
+  if (OP::IsDxilOpFuncCallInst(TextureHandleInst, OP::OpCode::AnnotateHandle)) {
+    DxilInst_AnnotateHandle AnnotateHandle(TextureHandleInst);
+    DxilResourceProperties ResourceProperties =
+        resource_helper::loadPropsFromAnnotateHandle(AnnotateHandle,
+                                                     *DM.GetShaderModel());
+    return ResourceProperties.getResourceClass() == DXIL::ResourceClass::SRV &&
+           (ResourceProperties.getResourceKind() ==
+                DXIL::ResourceKind::Texture2DMS ||
+            ResourceProperties.getResourceKind() ==
+                DXIL::ResourceKind::Texture2DMSArray);
+  }
 
-  // FP16 type doesn't have its own identity, and is covered by float type...
+  return false;
+}
 
-  auto TextureLoadOverloads = std::vector<Type *>{
-      Type::getFloatTy(Ctx), Type::getInt16Ty(Ctx), Type::getInt32Ty(Ctx)};
+bool DxilReduceMSAAToSingleSample::runOnModule(Module &M) {
+  DxilModule &DM = M.GetOrCreateDxilModule();
 
+  OP *HlslOP = DM.GetOP();
   bool Modified = false;
 
-  for (const auto &Overload : TextureLoadOverloads) {
+  // Iterate every materialised TextureLoad overload; the 16-bit form
+  // lowers Texture2DMS<half4>.Load.
+  for (const auto &TextureLoadOverload :
+       HlslOP->GetOpFuncList(DXIL::OpCode::TextureLoad)) {
+    Function *TexLoadFunction = TextureLoadOverload.second;
+    if (!TexLoadFunction)
+      continue;
 
-    Function *TexLoadFunction =
-        HlslOP->GetOpFunc(DXIL::OpCode::TextureLoad, Overload);
-    auto TexLoadFunctionUses = TexLoadFunction->uses();
-
-    for (auto FI = TexLoadFunctionUses.begin();
-         FI != TexLoadFunctionUses.end();) {
+    for (auto FI = TexLoadFunction->use_begin();
+         FI != TexLoadFunction->use_end();) {
       auto &FunctionUse = *FI++;
-      auto FunctionUser = FunctionUse.getUser();
-      auto instruction = cast<Instruction>(FunctionUser);
-      DxilInst_TextureLoad LoadInstruction(instruction);
-      auto TextureHandle = LoadInstruction.get_srv();
-      auto TextureHandleInst = cast<CallInst>(TextureHandle);
-      DxilInst_CreateHandle createHandle(TextureHandleInst);
-      // Dynamic rangeId is not supported
-      if (isa<ConstantInt>(createHandle.get_rangeId())) {
-        unsigned rangeId =
-            cast<ConstantInt>(createHandle.get_rangeId())->getLimitedValue();
-        if (static_cast<DXIL::ResourceClass>(
-                createHandle.get_resourceClass_val()) ==
-            DXIL::ResourceClass::SRV) {
-          auto Resource = DM.GetSRV(rangeId);
-          if (Resource.GetKind() == DXIL::ResourceKind::Texture2DMS ||
-              Resource.GetKind() == DXIL::ResourceKind::Texture2DMSArray) {
-            // "2" is the mip-level/sample-index operand index:
-            // https://github.com/Microsoft/DirectXShaderCompiler/blob/master/docs/DXIL.rst#textureload
-            instruction->setOperand(2, HlslOP->GetI32Const(0));
-            Modified = true;
-          }
-        }
+      auto *InstructionUser = dyn_cast<Instruction>(FunctionUse.getUser());
+      if (!InstructionUser)
+        continue;
+
+      DxilInst_TextureLoad LoadInstruction(InstructionUser);
+      if (!LoadInstruction)
+        continue;
+
+      if (IsMultisampledSRVHandle(LoadInstruction.get_srv(), DM)) {
+        LoadInstruction.set_mipLevelOrSampleCount(HlslOP->GetI32Const(0));
+        Modified = true;
       }
     }
   }

tools/clang/test/HLSLFileCheck/pix/constantcolorhalf.hlsl

@@ -0,0 +1,19 @@
+// RUN: %dxc -enable-16bit-types -Emain -Tps_6_2 %s | %opt -S -hlsl-dxil-constantColor,constant-red=0.5,constant-green=0.25,constant-blue=0.125,constant-alpha=1 | %FileCheck %s
+
+// A native half SV_Target lowers to dx.op.storeOutput.f16.
+
+// The override values are 0.5, 0.25, 0.125 and 1.0 as half:
+// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 0, half 0xH3800)
+// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 1, half 0xH3400)
+// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 2, half 0xH3000)
+// CHECK: call void @dx.op.storeOutput.f16(i32 5, i32 0, i32 0, i8 3, half 0xH3C00)
+
+// Unused storeOutput overloads must not remain as external declarations.
+// CHECK-NOT: declare void @dx.op.storeOutput.f32
+// CHECK-NOT: declare void @dx.op.storeOutput.i16
+// CHECK-NOT: declare void @dx.op.storeOutput.i32
+
+[RootSignature("")]
+half4 main() : SV_Target {
+    return half4(0, 0, 0, 0);
+}