Make sure status only used by CheckAccessFullyMapped. (#763)

* Make sure status only used by CheckAccessFullyMapped.

docs/DXIL.rst

@@ -2807,6 +2807,7 @@ INSTR.CALLOLOAD                        Call to DXIL intrinsic must match overloa
 INSTR.CANNOTPULLPOSITION               pull-model evaluation of position disallowed
 INSTR.CBUFFERCLASSFORCBUFFERHANDLE     Expect Cbuffer for CBufferLoad handle
 INSTR.CBUFFEROUTOFBOUND                Cbuffer access out of bound
+INSTR.CHECKACCESSFULLYMAPPED           CheckAccessFullyMapped should only used on resource status
 INSTR.COORDINATECOUNTFORRAWTYPEDBUF    raw/typed buffer don't need 2 coordinates
 INSTR.COORDINATECOUNTFORSTRUCTBUF      structured buffer require 2 coordinates
 INSTR.DXILSTRUCTUSER                   Dxil struct types should only used by ExtractValue
@@ -2857,6 +2858,7 @@ INSTR.SAMPLEINDEXFORLOAD2DMS           load on Texture2DMS/2DMSArray require sam
 INSTR.SAMPLERMODEFORLOD                lod instruction requires sampler declared in default mode
 INSTR.SAMPLERMODEFORSAMPLE             sample/_l/_d/_cl_s/gather instruction requires sampler declared in default mode
 INSTR.SAMPLERMODEFORSAMPLEC            sample_c_*/gather_c instructions require sampler declared in comparison mode
+INSTR.STATUS                           Resource status should only used by CheckAccessFullyMapped
 INSTR.STRUCTBITCAST                    Bitcast on struct types is not allowed
 INSTR.TEXTUREOFFSET                    offset texture instructions must take offset which can resolve to integer literal in the range -8 to 7
 INSTR.TGSMRACECOND                     Race condition writing to shared memory detected, consider making this write conditional

include/dxc/HLSL/DxilConstants.h

@@ -81,6 +81,8 @@ namespace DXIL {
   const float kMaxMipLodBias = 15.99f;
   const float kMinMipLodBias = -16.0f;
 
+  const unsigned kResRetStatusIndex = 4;
+
   enum class ComponentType : uint8_t { 
     Invalid = 0,
     I1, I16, U16, I32, U32, I64, U64,

include/dxc/HLSL/DxilOperations.h

@@ -56,6 +56,7 @@ public:
 
   llvm::Type *GetResRetType(llvm::Type *pOverloadType);
   llvm::Type *GetCBufferRetType(llvm::Type *pOverloadType);
+  bool IsResRetType(llvm::Type *Ty);
 
   // Try to get the opcode class for a function.
   // Return true and set `opClass` if the given function is a dxil function.

include/dxc/HLSL/DxilValidation.h

@@ -59,6 +59,7 @@ enum class ValidationRule : unsigned {
   InstrCBufferOutOfBound, // Cbuffer access out of bound
   InstrCallOload, // Call to DXIL intrinsic must match overload signature
   InstrCannotPullPosition, // pull-model evaluation of position disallowed
+  InstrCheckAccessFullyMapped, // CheckAccessFullyMapped should only used on resource status
   InstrCoordinateCountForRawTypedBuf, // raw/typed buffer don't need 2 coordinates
   InstrCoordinateCountForStructBuf, // structured buffer require 2 coordinates
   InstrDxilStructUser, // Dxil struct types should only used by ExtractValue
@@ -109,6 +110,7 @@ enum class ValidationRule : unsigned {
   InstrSamplerModeForLOD, // lod instruction requires sampler declared in default mode
   InstrSamplerModeForSample, // sample/_l/_d/_cl_s/gather instruction requires sampler declared in default mode
   InstrSamplerModeForSampleC, // sample_c_*/gather_c instructions require sampler declared in comparison mode
+  InstrStatus, // Resource status should only used by CheckAccessFullyMapped
   InstrStructBitCast, // Bitcast on struct types is not allowed
   InstrTGSMRaceCond, // Race condition writing to shared memory detected, consider making this write conditional
   InstrTextureOffset, // offset texture instructions must take offset which can resolve to integer literal in the range -8 to 7

lib/HLSL/DxilOperations.cpp

@@ -941,6 +941,14 @@ Type *OP::GetInt4Type() const {
   return m_pInt4Type;
 }
 
+bool OP::IsResRetType(llvm::Type *Ty) {
+  for (Type *ResTy : m_pResRetType) {
+    if (Ty == ResTy)
+      return true;
+  }
+  return false;
+}
+
 Type *OP::GetResRetType(Type *pOverloadType) {
   unsigned TypeSlot = GetTypeSlot(pOverloadType);
 

lib/HLSL/DxilValidation.cpp

@@ -115,6 +115,8 @@ const char *hlsl::GetValidationRuleText(ValidationRule value) {
     case hlsl::ValidationRule::InstrPtrBitCast: return "Pointer type bitcast must be have same size";
     case hlsl::ValidationRule::InstrMinPrecisonBitCast: return "Bitcast on minprecison types is not allowed";
     case hlsl::ValidationRule::InstrStructBitCast: return "Bitcast on struct types is not allowed";
+    case hlsl::ValidationRule::InstrStatus: return "Resource status should only used by CheckAccessFullyMapped";
+    case hlsl::ValidationRule::InstrCheckAccessFullyMapped: return "CheckAccessFullyMapped should only used on resource status";
     case hlsl::ValidationRule::InstrOpConst: return "%0 of %1 must be an immediate constant";
     case hlsl::ValidationRule::InstrAllowed: return "Instructions must be of an allowed type";
     case hlsl::ValidationRule::InstrOpCodeReserved: return "Instructions must not reference reserved opcodes";
@@ -808,7 +810,7 @@ struct ResRetUsage {
 };
 
 static void CollectGetDimResRetUsage(ResRetUsage &usage, Instruction *ResRet,
-                               ValidationContext &ValCtx) {
+                                     ValidationContext &ValCtx) {
   const unsigned kMaxResRetElementIndex = 5;
   for (User *U : ResRet->users()) {
     if (ExtractValueInst *EVI = dyn_cast<ExtractValueInst>(U)) {
@@ -826,7 +828,7 @@ static void CollectGetDimResRetUsage(ResRetUsage &usage, Instruction *ResRet,
         case 3:
           usage.w = true;
           break;
-        case 4:
+        case DXIL::kResRetStatusIndex:
           usage.status = true;
           break;
         default:
@@ -845,6 +847,36 @@ static void CollectGetDimResRetUsage(ResRetUsage &usage, Instruction *ResRet,
   }
 }
 
+static void ValidateStatus(Instruction *ResRet, ValidationContext &ValCtx) {
+  ResRetUsage usage;
+  CollectGetDimResRetUsage(usage, ResRet, ValCtx);
+  if (usage.status) {
+    for (User *U : ResRet->users()) {
+      if (ExtractValueInst *EVI = dyn_cast<ExtractValueInst>(U)) {
+        for (unsigned idx : EVI->getIndices()) {
+          switch (idx) {
+          case DXIL::kResRetStatusIndex:
+            for (User *SU : EVI->users()) {
+              Instruction *I = cast<Instruction>(SU);
+              // Make sure all use is CheckAccess.
+              if (!isa<CallInst>(I)) {
+                ValCtx.EmitInstrError(I, ValidationRule::InstrStatus);
+                return;
+              }
+              if (!ValCtx.DxilMod.GetOP()->IsDxilOpFuncCallInst(
+                      I, DXIL::OpCode::CheckAccessFullyMapped)) {
+                ValCtx.EmitInstrError(I, ValidationRule::InstrStatus);
+                return;
+              }
+            }
+            break;
+          }
+        }
+      }
+    }
+  }
+}
+
 static void ValidateResourceCoord(CallInst *CI, DXIL::ResourceKind resKind,
                                   ArrayRef<Value *> coords,
                                   ValidationContext &ValCtx) {
@@ -1518,6 +1550,21 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
         {sample.get_offset0(), sample.get_offset1(), sample.get_offset2()},
         /*IsSampleC*/ false, ValCtx);
   } break;
+  case DXIL::OpCode::CheckAccessFullyMapped: {
+    Value *Src = CI->getArgOperand(DXIL::OperandIndex::kUnarySrc0OpIdx);
+    ExtractValueInst *EVI = dyn_cast<ExtractValueInst>(Src);
+    if (!EVI) {
+      ValCtx.EmitInstrError(CI, ValidationRule::InstrCheckAccessFullyMapped);
+    } else {
+      Value *V = EVI->getOperand(0);
+      bool isLegal = EVI->getNumIndices() == 1 &&
+                     EVI->getIndices()[0] == DXIL::kResRetStatusIndex &&
+                     ValCtx.DxilMod.GetOP()->IsResRetType(V->getType());
+      if (!isLegal) {
+        ValCtx.EmitInstrError(CI, ValidationRule::InstrCheckAccessFullyMapped);
+      }
+    }
+  } break;
   case DXIL::OpCode::Barrier: {
     DxilInst_Barrier barrier(CI);
     Value *mode = barrier.get_barrierMode();

lib/HLSL/HLOperationLower.cpp

@@ -2292,10 +2292,20 @@ Value *ScalarizeElements(Type *RetTy, ArrayRef<Value*> Elts, IRBuilder<> &Builde
   return retVal;
 }
 
-void UpdateStatus(Value *ResRet, Value *status, IRBuilder<> &Builder) {
+void UpdateStatus(Value *ResRet, Value *status, IRBuilder<> &Builder,
+                  hlsl::OP *hlslOp) {
   if (status && !isa<UndefValue>(status)) {
-    Value *statusVal = Builder.CreateExtractValue(ResRet, 4);
-    Builder.CreateStore(statusVal, status);
+    Value *statusVal = Builder.CreateExtractValue(ResRet, DXIL::kResRetStatusIndex);
+    Value *checkAccessOp = hlslOp->GetI32Const(
+        static_cast<unsigned>(DXIL::OpCode::CheckAccessFullyMapped));
+    Function *checkAccessFn = hlslOp->GetOpFunc(
+        DXIL::OpCode::CheckAccessFullyMapped, statusVal->getType());
+    // CheckAccess on status.
+    Value *bStatus =
+        Builder.CreateCall(checkAccessFn, {checkAccessOp, statusVal});
+    Value *extStatus =
+        Builder.CreateZExt(bStatus, Type::getInt32Ty(status->getContext()));
+    Builder.CreateStore(extStatus, status);
   }
 }
 
@@ -2481,8 +2491,19 @@ Value *TranslateCalculateLOD(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
   return LOD;
 }
 
+Value *TranslateCheckAccess(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
+                            HLOperationLowerHelper &helper,
+                            HLObjectOperationLowerHelper *pObjHelper,
+                            bool &Translated) {
+  // Translate CheckAccess into uint->bool, later optimization should remove it.
+  // Real checkaccess is generated in UpdateStatus.
+  IRBuilder<> Builder(CI);
+  Value *V = CI->getArgOperand(HLOperandIndex::kUnaryOpSrc0Idx);
+  return Builder.CreateTrunc(V, helper.i1Ty);
+}
+
 void GenerateDxilSample(CallInst *CI, Function *F, ArrayRef<Value *> sampleArgs,
-                        Value *status) {
+                        Value *status, hlsl::OP *hlslOp) {
   IRBuilder<> Builder(CI);
 
   CallInst *call = Builder.CreateCall(F, sampleArgs);
@@ -2495,7 +2516,7 @@ void GenerateDxilSample(CallInst *CI, Function *F, ArrayRef<Value *> sampleArgs,
 
   // get status
   if (status) {
-    UpdateStatus(call, status, Builder);
+    UpdateStatus(call, status, Builder, hlslOp);
   }
 }
 
@@ -2525,7 +2546,7 @@ Value *TranslateSample(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
         sampleHelper.offset[0], sampleHelper.offset[1], sampleHelper.offset[2],
         // Clamp.
         sampleHelper.clamp};
-    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status);
+    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status, hlslOP);
   } break;
   case OP::OpCode::SampleLevel: {
     Value *sampleArgs[] = {
@@ -2537,7 +2558,7 @@ Value *TranslateSample(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
         sampleHelper.offset[0], sampleHelper.offset[1], sampleHelper.offset[2],
         // LOD.
         sampleHelper.special};
-    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status);
+    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status, hlslOP);
   } break;
   case OP::OpCode::SampleGrad: {
     Value *sampleArgs[] = {
@@ -2553,7 +2574,7 @@ Value *TranslateSample(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
         sampleHelper.ddy[0], sampleHelper.ddy[1], sampleHelper.ddy[2],
         // Clamp.
         sampleHelper.clamp};
-    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status);
+    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status, hlslOP);
   } break;
   case OP::OpCode::SampleBias: {
     // Clamp bias for immediate.
@@ -2576,7 +2597,7 @@ Value *TranslateSample(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
         bias,
         // Clamp.
         sampleHelper.clamp};
-    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status);
+    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status, hlslOP);
   } break;
   case OP::OpCode::SampleCmp: {
     Value *sampleArgs[] = {
@@ -2590,7 +2611,7 @@ Value *TranslateSample(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
         sampleHelper.special,
         // Clamp.
         sampleHelper.clamp};
-    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status);
+    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status, hlslOP);
   } break;
   case OP::OpCode::SampleCmpLevelZero:
   default: {
@@ -2604,7 +2625,7 @@ Value *TranslateSample(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
         sampleHelper.offset[0], sampleHelper.offset[1], sampleHelper.offset[2],
         // CmpVal.
         sampleHelper.special};
-    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status);
+    GenerateDxilSample(CI, F, sampleArgs, sampleHelper.status, hlslOP);
   } break;
   }
   // CI is replaced in GenerateDxilSample.
@@ -2773,7 +2794,7 @@ GatherHelper::GatherHelper(
 
 void GenerateDxilGather(CallInst *CI, Function *F,
                         MutableArrayRef<Value *> gatherArgs,
-                        GatherHelper &helper) {
+                        GatherHelper &helper, hlsl::OP *hlslOp) {
   IRBuilder<> Builder(CI);
 
   CallInst *call = Builder.CreateCall(F, gatherArgs);
@@ -2809,7 +2830,7 @@ void GenerateDxilGather(CallInst *CI, Function *F,
   }
   // Get status
   if (helper.status) {
-    UpdateStatus(call, helper.status, Builder);
+    UpdateStatus(call, helper.status, Builder, hlslOp);
   }
 }
 
@@ -2867,7 +2888,7 @@ Value *TranslateGather(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
         gatherHelper.offset[0], gatherHelper.offset[1],
         // Channel.
         channelArg};
-    GenerateDxilGather(CI, F, gatherArgs, gatherHelper);
+    GenerateDxilGather(CI, F, gatherArgs, gatherHelper, hlslOP);
   } break;
   case OP::OpCode::TextureGatherCmp: {
     Value *gatherArgs[] = {
@@ -2881,7 +2902,7 @@ Value *TranslateGather(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
         channelArg,
         // CmpVal.
         gatherHelper.special};
-    GenerateDxilGather(CI, F, gatherArgs, gatherHelper);
+    GenerateDxilGather(CI, F, gatherArgs, gatherHelper, hlslOP);
   } break;
   default:
     DXASSERT(0, "invalid opcode for Gather");
@@ -3157,7 +3178,7 @@ void TranslateLoad(ResLoadHelper &helper, HLResource::Kind RK,
   // Save new ret val.
   helper.retVal = retValNew;
   // get status
-  UpdateStatus(ResRet, helper.status, Builder);
+  UpdateStatus(ResRet, helper.status, Builder, OP);
 }
 
 Value *TranslateResourceLoad(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode,
@@ -4155,7 +4176,7 @@ IntrinsicLower gLowerTable[static_cast<unsigned>(IntrinsicOp::Num_Intrinsics)] =
     {IntrinsicOp::IOP_AddUint64,  TranslateAddUint64,  DXIL::OpCode::UAddc},
     {IntrinsicOp::IOP_AllMemoryBarrier, TrivialBarrier, DXIL::OpCode::Barrier},
     {IntrinsicOp::IOP_AllMemoryBarrierWithGroupSync, TrivialBarrier, DXIL::OpCode::Barrier},
-    {IntrinsicOp::IOP_CheckAccessFullyMapped, TrivialUnaryOperation, DXIL::OpCode::CheckAccessFullyMapped},
+    {IntrinsicOp::IOP_CheckAccessFullyMapped, TranslateCheckAccess, DXIL::OpCode::CheckAccessFullyMapped},
     {IntrinsicOp::IOP_D3DCOLORtoUBYTE4, TranslateD3DColorToUByte4, DXIL::OpCode::NumOpCodes},
     {IntrinsicOp::IOP_DeviceMemoryBarrier, TrivialBarrier, DXIL::OpCode::Barrier},
     {IntrinsicOp::IOP_DeviceMemoryBarrierWithGroupSync, TrivialBarrier, DXIL::OpCode::Barrier},
@@ -5408,7 +5429,7 @@ void GenerateStructBufLd(Value *handle, Value *bufIdx, Value *offset,
     }
 
     // status
-    UpdateStatus(Ld, status, Builder);
+    UpdateStatus(Ld, status, Builder, OP);
     return;
   } else {
     // 64 bit.
@@ -5434,7 +5455,7 @@ void GenerateStructBufLd(Value *handle, Value *bufIdx, Value *offset,
     Make64bitResultForLoad(EltTy, resultElts32, size, resultElts, OP, Builder);
 
     // status
-    UpdateStatus(Ld, status, Builder);
+    UpdateStatus(Ld, status, Builder, OP);
 
     return;
   }

tools/clang/test/CodeGenHLSL/quick-test/check_status.hlsl

@@ -0,0 +1,15 @@
+// RUN: %dxc -E main -T cs_6_0 %s | FileCheck %s
+
+// Make sure checkaccess is generated.
+// CHECK: dx.op.checkAccessFullyMapped.i32(i32 71
+
+RWBuffer<float4> g_buffer;
+RWBuffer<uint> g_result;
+
+[numthreads(1, 1, 1)]
+void main()
+{
+    uint res;
+    float4 data = g_buffer.Load(0, res);
+    g_result[0] = res;
+}

tools/clang/test/CodeGenHLSL/quick-test/check_status2.hlsl

@@ -0,0 +1,15 @@
+// RUN: %dxc -E main -T cs_6_0 %s | FileCheck %s
+
+// Make sure no checkaccess is generated.
+// CHECK-NOT: dx.op.checkAccessFullyMapped.i32(i32 71
+
+RWBuffer<float4> g_buffer;
+RWBuffer<uint> g_result;
+
+[numthreads(1, 1, 1)]
+void main()
+{
+    uint res;
+    float4 data = g_buffer.Load(0, res);
+    g_result[0] = 1;
+}

tools/clang/test/CodeGenHLSL/quick-test/check_status3.hlsl

@@ -0,0 +1,16 @@
+// RUN: %dxc -E main -T cs_6_0 %s | FileCheck %s
+
+// Make sure only 1 checkaccess is generated.
+// CHECK: dx.op.checkAccessFullyMapped.i32(i32 71
+// CHECK-NOT: dx.op.checkAccessFullyMapped.i32(i32 71
+
+RWBuffer<float4> g_buffer;
+RWBuffer<uint> g_result;
+
+[numthreads(1, 1, 1)]
+void main()
+{
+    uint res;
+    float4 data = g_buffer.Load(0, res);
+    g_result[0] = CheckAccessFullyMapped(res);
+}

utils/hct/hctdb.py

@@ -1603,6 +1603,8 @@ class db_dxil(object):
         self.add_valrule("Instr.PtrBitCast", "Pointer type bitcast must be have same size")
         self.add_valrule("Instr.MinPrecisonBitCast", "Bitcast on minprecison types is not allowed")
         self.add_valrule("Instr.StructBitCast", "Bitcast on struct types is not allowed")
+        self.add_valrule("Instr.Status", "Resource status should only used by CheckAccessFullyMapped")
+        self.add_valrule("Instr.CheckAccessFullyMapped", "CheckAccessFullyMapped should only used on resource status")
         self.add_valrule_msg("Instr.OpConst", "DXIL intrinsic requires an immediate constant operand", "%0 of %1 must be an immediate constant")
         self.add_valrule("Instr.Allowed", "Instructions must be of an allowed type")
         self.add_valrule("Instr.OpCodeReserved", "Instructions must not reference reserved opcodes")