Browse Source

Rework PIX access tracking to remove hard coded table.

Instead of hard-coding properties here, constructing every overload of
every resource method we know about, then iterating users, we:
- iterate through used intrinsic functions, collecting the ones that
  have handle parameters (resource accesses).  This will pick up all
  used overloads.
- Instead of hard coding read/write access in a table here, we use the
  function attribute which carries this same information for main resource
  operations.
- Exceptions to matching function attribute:
  GetDimensions: DescriptorRead, but commented to match prior behavior
  BufferUpdateCounter: Counter
  TraceRay[Inline]: Read, but commented to match prior behavior for now
- Then, this type of resource access is emitted for the first handle arg
- Subsequent handles are considered DescriptorRead operations,
- DescriptorRead currently just aliases to Read, but could be useful in
  the future if we want to differentiate or support GetDimensions.
Tex Riddell 6 năm trước cách đây
mục cha
commit
7a085056f8

+ 1 - 0
include/dxc/DXIL/DxilOperations.h

@@ -89,6 +89,7 @@ public:
   static const char *GetAtomicOpName(DXIL::AtomicBinOpCode OpCode);
   static OpCodeClass GetOpCodeClass(OpCode OpCode);
   static const char *GetOpCodeClassName(OpCode OpCode);
+  static llvm::Attribute::AttrKind GetMemAccessAttr(OpCode opCode);
   static bool IsOverloadLegal(OpCode OpCode, llvm::Type *pType);
   static bool CheckOpCodeTable();
   static bool IsDxilOpFuncName(llvm::StringRef name);

+ 5 - 0
lib/DXIL/DxilOperations.cpp

@@ -474,6 +474,11 @@ const char *OP::GetOpCodeClassName(OpCode opCode) {
   return m_OpCodeProps[(unsigned)opCode].pOpCodeClassName;
 }
 
+llvm::Attribute::AttrKind OP::GetMemAccessAttr(OpCode opCode) {
+  DXASSERT(0 <= (unsigned)opCode && opCode < OpCode::NumOpCodes, "otherwise caller passed OOB index");
+  return m_OpCodeProps[(unsigned)opCode].FuncAttr;
+}
+
 bool OP::IsOverloadLegal(OpCode opCode, Type *pType) {
   DXASSERT(0 <= (unsigned)opCode && opCode < OpCode::NumOpCodes, "otherwise caller passed OOB index");
   unsigned TypeSlot = GetTypeSlot(pType);

+ 56 - 70
lib/DxilPIXPasses/DxilShaderAccessTracking.cpp

@@ -48,7 +48,12 @@ enum class ShaderAccessFlags : uint32_t
 
   // "Counter" access is only applicable to UAVs; it means the counter buffer attached to the UAV
   // was accessed, but not necessarily the UAV resource.
-  Counter = 1 << 2
+  Counter = 1 << 2,
+
+  // Descriptor-only read (if any), but not the resource contents (if any).
+  // Used for GetDimensions, samplers, and secondary texture for sampler feedback.
+  // TODO: Make this a unique value if supported in PIX, then enable GetDimensions
+  DescriptorRead = 1 << 0,
 };
 
 // This enum doesn't have to match PIX's version, because the values are received from PIX encoded in ASCII.
@@ -435,80 +440,61 @@ bool DxilShaderAccessTracking::runOnModule(Module &M)
       DM.ReEmitDxilResources();
     }
 
-    struct ResourceAccessFunction
-    {
-      DXIL::OpCode opcode;
-      ShaderAccessFlags readWrite;
-      ArrayRef<Type*> overloads;
-    };
-
-    Type* voidType[] = { Type::getVoidTy(Ctx) };
-    Type* i32[] = { Type::getInt32Ty(Ctx) };
-    Type* f16f32[] = { Type::getHalfTy(Ctx), Type::getFloatTy(Ctx) };
-    Type* f32i32[] = { Type::getFloatTy(Ctx), Type::getInt32Ty(Ctx) };
-    Type* f32i32f64[] = { Type::getFloatTy(Ctx), Type::getInt32Ty(Ctx), Type::getDoubleTy(Ctx) };
-    Type* f16f32i16i32[] = { Type::getHalfTy(Ctx), Type::getFloatTy(Ctx), Type::getInt16Ty(Ctx), Type::getInt32Ty(Ctx) };
-    Type* f16f32f64i16i32i64[] = { Type::getHalfTy(Ctx), Type::getFloatTy(Ctx), Type::getDoubleTy(Ctx), Type::getInt16Ty(Ctx), Type::getInt32Ty(Ctx), Type::getInt64Ty(Ctx) };
-
-
-    // todo: should "GetDimensions" mean a resource access?
-    static_assert(DXIL::OpCode::NumOpCodes == static_cast<DXIL::OpCode>(214), "Please update PIX passes if any resource access opcodes are added");
-    ResourceAccessFunction raFunctions[] = {
-      { DXIL::OpCode::CBufferLoadLegacy             , ShaderAccessFlags::Read   , f32i32f64 },
-      { DXIL::OpCode::CBufferLoad                   , ShaderAccessFlags::Read   , f16f32f64i16i32i64 },
-      { DXIL::OpCode::Sample                        , ShaderAccessFlags::Read   , f16f32 },
-      { DXIL::OpCode::SampleBias                    , ShaderAccessFlags::Read   , f16f32 },
-      { DXIL::OpCode::SampleLevel                   , ShaderAccessFlags::Read   , f16f32 },
-      { DXIL::OpCode::SampleGrad                    , ShaderAccessFlags::Read   , f16f32 },
-      { DXIL::OpCode::SampleCmp                     , ShaderAccessFlags::Read   , f16f32 },
-      { DXIL::OpCode::SampleCmpLevelZero            , ShaderAccessFlags::Read   , f16f32 },
-      { DXIL::OpCode::TextureLoad                   , ShaderAccessFlags::Read   , f16f32i16i32 },
-      { DXIL::OpCode::TextureStore                  , ShaderAccessFlags::Write  , f16f32i16i32 },
-      { DXIL::OpCode::TextureGather                 , ShaderAccessFlags::Read   , f16f32i16i32 },
-      { DXIL::OpCode::TextureGatherCmp              , ShaderAccessFlags::Read   , f16f32i16i32 },
-      { DXIL::OpCode::BufferLoad                    , ShaderAccessFlags::Read   , f32i32 },
-      { DXIL::OpCode::RawBufferLoad                 , ShaderAccessFlags::Read   , f16f32i16i32 },
-      { DXIL::OpCode::RawBufferStore                , ShaderAccessFlags::Write  , f16f32i16i32 },
-      { DXIL::OpCode::BufferStore                   , ShaderAccessFlags::Write  , f32i32 },
-      { DXIL::OpCode::BufferUpdateCounter           , ShaderAccessFlags::Counter, voidType },
-      { DXIL::OpCode::AtomicBinOp                   , ShaderAccessFlags::Write  , i32 },
-      { DXIL::OpCode::AtomicCompareExchange         , ShaderAccessFlags::Write  , i32 },
-      { DXIL::OpCode::WriteSamplerFeedback          , ShaderAccessFlags::Write  , voidType },
-      { DXIL::OpCode::WriteSamplerFeedbackBias      , ShaderAccessFlags::Write  , voidType },
-      { DXIL::OpCode::WriteSamplerFeedbackGrad      , ShaderAccessFlags::Write  , voidType },
-      { DXIL::OpCode::WriteSamplerFeedbackLevel     , ShaderAccessFlags::Write  , voidType },
-    };
-
-    for (const auto & raFunction : raFunctions) {
-      for (const auto & Overload : raFunction.overloads) {
-        Function * TheFunction = HlslOP->GetOpFunc(raFunction.opcode, Overload);
-        auto TexLoadFunctionUses = TheFunction->uses();
-        for (auto FI = TexLoadFunctionUses.begin(); FI != TexLoadFunctionUses.end(); ) {
-          auto & FunctionUse = *FI++;
-          auto FunctionUser = FunctionUse.getUser();
-          auto Call = cast<CallInst>(FunctionUser);
-
-          auto res = GetResourceFromHandle(Call->getArgOperand(1), DM);
+    for (llvm::Function & F : M.functions()) {
+      // Only used DXIL intrinsics:
+      if (!F.isDeclaration() || F.isIntrinsic() || F.use_empty() || !OP::IsDxilOpFunc(&F))
+        continue;
+
+      // Gather handle parameter indices, if any
+      FunctionType *fnTy = cast<FunctionType>(F.getType()->getPointerElementType());
+      SmallVector<unsigned, 4> handleParams;
+      for (unsigned iParam = 1; iParam < fnTy->getFunctionNumParams(); ++iParam) {
+        if (fnTy->getParamType(iParam) == HlslOP->GetHandleType())
+          handleParams.push_back(iParam);
+      }
+      if (handleParams.empty())
+        continue;
+
+      auto FunctionUses = F.uses();
+      for (auto FI = FunctionUses.begin(); FI != FunctionUses.end(); ) {
+        auto & FunctionUse = *FI++;
+        auto FunctionUser = FunctionUse.getUser();
+        auto Call = cast<CallInst>(FunctionUser);
+        auto opCode = OP::GetDxilOpFuncCallInst(Call);
+
+        // Base Read/Write on function attribute - should match for all normal resource operations
+        ShaderAccessFlags readWrite = ShaderAccessFlags::Write;
+        if (OP::GetMemAccessAttr(opCode) == llvm::Attribute::AttrKind::ReadOnly)
+          readWrite = ShaderAccessFlags::Read;
+
+        // Special cases
+        switch (opCode) {
+        case DXIL::OpCode::GetDimensions:
+          // readWrite = ShaderAccessFlags::DescriptorRead;  // TODO: Support GetDimensions
+          continue;
+        case DXIL::OpCode::BufferUpdateCounter:
+          readWrite = ShaderAccessFlags::Counter;
+          break;
+        case DXIL::OpCode::TraceRay:
+        case DXIL::OpCode::RayQuery_TraceRayInline:
+          // Read of AccelerationStructure; doesn't match function attribute
+          // readWrite = ShaderAccessFlags::Read;  // TODO: Support TraceRay[Inline]
+          continue;
+        default:
+          break;
+        }
 
+        for (unsigned iParam : handleParams) {
+          auto res = GetResourceFromHandle(Call->getArgOperand(iParam), DM);
           // Don't instrument the accesses to the UAV that we just added
-          if (res.resource->GetSpaceID() == (unsigned)-2) {
-            continue;
+          if (res.resClass == DXIL::ResourceClass::UAV && res.resource->GetSpaceID() == (unsigned)-2) {
+            break;
           }
-
-          if (EmitResourceAccess(res, Call, HlslOP, Ctx, raFunction.readWrite)) {
+          if (EmitResourceAccess(res, Call, HlslOP, Ctx, readWrite)) {
             Modified = true;
           }
-
-          // Find any secondary resource operand, typically a sampler, and mark it as read.
-          for (unsigned i = 2; i < Call->getNumArgOperands(); ++i) {
-            Value *Arg = Call->getOperand(i);
-            if (Arg->getType() == HlslOP->GetHandleType()) {
-              DxilResourceAndClass SecondaryResource = GetResourceFromHandle(Arg, DM);
-              if (EmitResourceAccess(SecondaryResource, Call, HlslOP, Ctx, ShaderAccessFlags::Read)) {
-                Modified = true;
-              }
-            }
-          }
+          // Remaining resources are DescriptorRead.
+          readWrite = ShaderAccessFlags::DescriptorRead;
         }
       }
     }