Browse Source

Support resource select for lib profile. (#940)

Xiang Li 7 years ago
parent
commit
b816c124f1

+ 1 - 0
docs/DXIL.rst

@@ -2817,6 +2817,7 @@ 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.CREATEHANDLEIMMRANGEID           Local resource must map to global resource.
 INSTR.DXILSTRUCTUSER                   Dxil struct types should only used by ExtractValue
 INSTR.DXILSTRUCTUSEROUTOFBOUND         Index out of bound when extract value from dxil struct types
 INSTR.EVALINTERPOLATIONMODE            Interpolation mode on %0 used with eval_* instruction must be linear, linear_centroid, linear_noperspective, linear_noperspective_centroid, linear_sample or linear_noperspective_sample

+ 1 - 0
include/dxc/HLSL/DxilValidation.h

@@ -63,6 +63,7 @@ enum class ValidationRule : unsigned {
   InstrCheckAccessFullyMapped, // CheckAccessFullyMapped should only used on resource status
   InstrCoordinateCountForRawTypedBuf, // raw/typed buffer don't need 2 coordinates
   InstrCoordinateCountForStructBuf, // structured buffer require 2 coordinates
+  InstrCreateHandleImmRangeID, // Local resource must map to global resource.
   InstrDxilStructUser, // Dxil struct types should only used by ExtractValue
   InstrDxilStructUserOutOfBound, // Index out of bound when extract value from dxil struct types
   InstrEvalInterpolationMode, // Interpolation mode on %0 used with eval_* instruction must be linear, linear_centroid, linear_noperspective, linear_noperspective_centroid, linear_sample or linear_noperspective_sample

+ 74 - 10
lib/HLSL/DxilCondenseResources.cpp

@@ -456,6 +456,69 @@ void DxilCondenseResources::PatchCreateHandle(DxilModule &DM) {
   }
 }
 
+static Value *PatchRangeIDForLib(DxilModule &DM, IRBuilder<> &Builder,
+                                 Value *rangeIdVal,
+                                 std::unordered_map<PHINode *, Value *> &phiMap,
+                                 DXIL::ResourceClass ResClass) {
+  Value *linkRangeID = nullptr;
+  if (isa<ConstantInt>(rangeIdVal)) {
+    unsigned rangeId = cast<ConstantInt>(rangeIdVal)->getLimitedValue();
+
+    const DxilModule::ResourceLinkInfo &linkInfo =
+        DM.GetResourceLinkInfo(ResClass, rangeId);
+    linkRangeID = Builder.CreateLoad(linkInfo.ResRangeID);
+  } else {
+    if (PHINode *phi = dyn_cast<PHINode>(rangeIdVal)) {
+      auto it = phiMap.find(phi);
+      if (it == phiMap.end()) {
+        unsigned numOperands = phi->getNumOperands();
+
+        PHINode *phiRangeID = Builder.CreatePHI(phi->getType(), numOperands);
+        phiMap[phi] = phiRangeID;
+
+        std::vector<Value *> rangeIDs(numOperands);
+        for (unsigned i = 0; i < numOperands; i++) {
+          Value *V = phi->getOperand(i);
+          BasicBlock *BB = phi->getIncomingBlock(i);
+          IRBuilder<> Builder(BB->getTerminator());
+          rangeIDs[i] = PatchRangeIDForLib(DM, Builder, V, phiMap, ResClass);
+        }
+
+        for (unsigned i = 0; i < numOperands; i++) {
+          Value *V = rangeIDs[i];
+          BasicBlock *BB = phi->getIncomingBlock(i);
+          phiRangeID->addIncoming(V, BB);
+        }
+        linkRangeID = phiRangeID;
+      } else {
+        linkRangeID = it->second;
+      }
+    } else if (SelectInst *si = dyn_cast<SelectInst>(rangeIdVal)) {
+      IRBuilder<> Builder(si);
+      Value *trueVal =
+          PatchRangeIDForLib(DM, Builder, si->getTrueValue(), phiMap, ResClass);
+      Value *falseVal = PatchRangeIDForLib(DM, Builder, si->getFalseValue(),
+                                           phiMap, ResClass);
+      linkRangeID = Builder.CreateSelect(si->getCondition(), trueVal, falseVal);
+    } else if (CastInst *cast = dyn_cast<CastInst>(rangeIdVal)) {
+      if (cast->getOpcode() == CastInst::CastOps::ZExt &&
+          cast->getOperand(0)->getType() == Type::getInt1Ty(DM.GetCtx())) {
+        // select cond, 1, 0.
+        IRBuilder<> Builder(cast);
+        Value *trueVal = PatchRangeIDForLib(
+            DM, Builder, ConstantInt::get(cast->getType(), 1), phiMap,
+            ResClass);
+        Value *falseVal = PatchRangeIDForLib(
+            DM, Builder, ConstantInt::get(cast->getType(), 0), phiMap,
+            ResClass);
+        linkRangeID =
+            Builder.CreateSelect(cast->getOperand(0), trueVal, falseVal);
+      }
+    }
+  }
+  return linkRangeID;
+}
+
 void DxilCondenseResources::PatchCreateHandleForLib(DxilModule &DM) {
   Function *createHandle = DM.GetOP()->GetOpFunc(DXIL::OpCode::CreateHandle,
                                                  Type::getVoidTy(DM.GetCtx()));
@@ -467,22 +530,23 @@ void DxilCondenseResources::PatchCreateHandleForLib(DxilModule &DM) {
 
     DXIL::ResourceClass ResClass =
         static_cast<DXIL::ResourceClass>(createHandle.get_resourceClass_val());
+
+    std::unordered_map<PHINode *, Value*> phiMap;
+    Value *rangeID = createHandle.get_rangeId();
+    IRBuilder<> Builder(handle);
+    Value *linkRangeID = PatchRangeIDForLib(
+        DM, Builder, rangeID, phiMap, ResClass);
+
     // Dynamic rangeId is not supported - skip and let validation report the
     // error.
-    if (!isa<ConstantInt>(createHandle.get_rangeId()))
+    if (!linkRangeID)
       continue;
-
-    unsigned rangeId =
-        cast<ConstantInt>(createHandle.get_rangeId())->getLimitedValue();
-
-    const DxilModule::ResourceLinkInfo &linkInfo =
-        DM.GetResourceLinkInfo(ResClass, rangeId);
-
-    IRBuilder<> Builder(handle);
-    Value *linkRangeID = Builder.CreateLoad(linkInfo.ResRangeID);
     // Update rangeID to linkinfo rangeID.
     handle->setArgOperand(DXIL::OperandIndex::kCreateHandleResIDOpIdx,
                           linkRangeID);
+    if (rangeID->user_empty() && isa<Instruction>(rangeID)) {
+      cast<Instruction>(rangeID)->eraseFromParent();
+    }
   }
 }
 

+ 3 - 0
lib/HLSL/DxilValidation.cpp

@@ -177,6 +177,7 @@ const char *hlsl::GetValidationRuleText(ValidationRule value) {
     case hlsl::ValidationRule::InstrExtractValue: return "ExtractValue should only be used on dxil struct types and cmpxchg";
     case hlsl::ValidationRule::InstrTGSMRaceCond: return "Race condition writing to shared memory detected, consider making this write conditional";
     case hlsl::ValidationRule::InstrAttributeAtVertexNoInterpolation: return "Attribute %0 must have nointerpolation mode in order to use GetAttributeAtVertex function.";
+    case hlsl::ValidationRule::InstrCreateHandleImmRangeID: return "Local resource must map to global resource.";
     case hlsl::ValidationRule::TypesNoVector: return "Vector type '%0' is not allowed";
     case hlsl::ValidationRule::TypesDefined: return "Type '%0' is not defined on DXIL primitives";
     case hlsl::ValidationRule::TypesIntWidth: return "Int type '%0' has an invalid width";
@@ -762,6 +763,8 @@ static DXIL::ResourceKind GetResourceKindAndCompTy(Value *handle, DXIL::Componen
 
   Value *rangeIndex = createHandle.get_rangeId();
   if (!isa<ConstantInt>(rangeIndex)) {
+    ValCtx.EmitInstrError(cast<CallInst>(handle),
+                          ValidationRule::InstrCreateHandleImmRangeID);
     // must be constant
     return DXIL::ResourceKind::Invalid;
   }

+ 19 - 0
tools/clang/test/CodeGenHLSL/lib_select_res.hlsl

@@ -0,0 +1,19 @@
+// RUN: %dxc -T lib_6_1 %s | FileCheck %s
+
+// Make sure load resource rangeID when select resource.
+// CHECK:load i32, i32* @ReadBuffer1_rangeID
+// CHECK:load i32, i32* @ReadBuffer_rangeID
+
+RWByteAddressBuffer outputBuffer : register(u0);
+ByteAddressBuffer ReadBuffer : register(t0);
+ByteAddressBuffer ReadBuffer1 : register(t1);
+
+void test( uint cond)
+{
+	ByteAddressBuffer buffer = ReadBuffer;
+        if (cond > 2)
+           buffer = ReadBuffer1;
+
+	uint v= buffer.Load(0);
+    outputBuffer.Store(0, v);
+}

+ 8 - 0
tools/clang/test/CodeGenHLSL/lib_select_res_entry.hlsl

@@ -0,0 +1,8 @@
+
+void test( uint cond);
+
+[shader("pixel")]
+float main(uint c : C) : SV_Target {
+   test(c);
+   return 1;
+}

+ 19 - 0
tools/clang/test/CodeGenHLSL/quick-test/lib_select_res.hlsl

@@ -0,0 +1,19 @@
+// RUN: %dxc -T lib_6_1 %s | FileCheck %s
+
+// Make sure load resource rangeID when select resource.
+// CHECK:load i32, i32* @ReadBuffer1_rangeID
+// CHECK:load i32, i32* @ReadBuffer_rangeID
+
+RWByteAddressBuffer outputBuffer : register(u0);
+ByteAddressBuffer ReadBuffer : register(t0);
+ByteAddressBuffer ReadBuffer1 : register(t1);
+
+void test( uint cond)
+{
+	ByteAddressBuffer buffer = ReadBuffer;
+        if (cond > 2)
+           buffer = ReadBuffer1;
+
+	uint v= buffer.Load(0);
+    outputBuffer.Store(0, v);
+}

+ 3 - 0
tools/clang/tools/dotnetc/EditorForm.cs

@@ -489,6 +489,9 @@ namespace MainNs
             if (target != null && target.Length == 6)
                 if (Int32.TryParse(new string(target[3], 1), out major))
                     return major >= 6;
+            if (target.StartsWith("lib"))
+                if (Int32.TryParse(new string(target[4], 1), out major))
+                    return major >= 6;
             return false;
         }
 

+ 20 - 0
tools/clang/unittests/HLSL/LinkerTest.cpp

@@ -47,6 +47,7 @@ public:
   TEST_METHOD(RunLinkFailReDefineGlobal);
   TEST_METHOD(RunLinkFailProfileMismatch);
   TEST_METHOD(RunLinkFailEntryNoProps);
+  TEST_METHOD(RunLinkFailSelectRes);
 
 
   dxc::DxcDllSupport m_dllSupport;
@@ -278,3 +279,22 @@ TEST_F(LinkerTest, RunLinkNoAlloca) {
 
   Link(L"ps_main", L"ps_6_0", pLinker, {libName, libName2}, {}, {"alloca"});
 }
+
+TEST_F(LinkerTest, RunLinkFailSelectRes) {
+  CComPtr<IDxcBlob> pEntryLib;
+  CompileLib(L"..\\CodeGenHLSL\\lib_select_res_entry.hlsl", &pEntryLib);
+  CComPtr<IDxcBlob> pLib;
+  CompileLib(L"..\\CodeGenHLSL\\lib_select_res.hlsl", &pLib);
+
+  CComPtr<IDxcLinker> pLinker;
+  CreateLinker(&pLinker);
+
+  LPCWSTR libName = L"main";
+  RegisterDxcModule(libName, pEntryLib, pLinker);
+
+  LPCWSTR libName2 = L"test";
+  RegisterDxcModule(libName2, pLib, pLinker);
+
+  LinkCheckMsg(L"main", L"ps_6_0", pLinker, {libName, libName2},
+               {"Local resource must map to global resource"});
+}

+ 1 - 0
utils/hct/hctdb.py

@@ -1713,6 +1713,7 @@ class db_dxil(object):
         self.add_valrule("Instr.ExtractValue", "ExtractValue should only be used on dxil struct types and cmpxchg")
         self.add_valrule("Instr.TGSMRaceCond", "Race condition writing to shared memory detected, consider making this write conditional")
         self.add_valrule("Instr.AttributeAtVertexNoInterpolation", "Attribute %0 must have nointerpolation mode in order to use GetAttributeAtVertex function.")
+        self.add_valrule("Instr.CreateHandleImmRangeID", "Local resource must map to global resource.")
 
         # Some legacy rules:
         # - space is only supported for shader targets 5.1 and higher