Browse Source

Disable race condition check (#3)

* Write thread group uniform value will not cause race condition.

* Only report race condition when know the data is not uniform.

* Disable race condition check.
Xiang Li 8 years ago
parent
commit
7b998429f3

+ 19 - 14
lib/HLSL/DxilValidation.cpp

@@ -2616,26 +2616,30 @@ static void ValidateGlobalVariable(GlobalVariable &GV,
   }
 }
 
-static void
-CollectFixAddressAccess(Value *V, std::vector<Instruction *> &fixAddrTGSMList) {
+static void CollectFixAddressAccess(Value *V,
+                                    std::vector<StoreInst *> &fixAddrTGSMList) {
   for (User *U : V->users()) {
     if (GEPOperator *GEP = dyn_cast<GEPOperator>(U)) {
       if (isa<ConstantExpr>(GEP) || GEP->hasAllConstantIndices()) {
         CollectFixAddressAccess(GEP, fixAddrTGSMList);
       }
-    } else if (isa<StoreInst>(U)) {
-      fixAddrTGSMList.emplace_back(cast<Instruction>(U));
+    } else if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
+      fixAddrTGSMList.emplace_back(SI);
     }
   }
 }
 
-static void
-ValidateTGSMRaceCondition(std::vector<Instruction *> &fixAddrTGSMList,
-                          ValidationContext &ValCtx) {
+static bool IsDivergent(Value *V) {
+  // TODO: return correct result.
+  return false;
+}
+
+static void ValidateTGSMRaceCondition(std::vector<StoreInst *> &fixAddrTGSMList,
+                                      ValidationContext &ValCtx) {
   std::unordered_set<Function *> fixAddrTGSMFuncSet;
-  for (Instruction *I : fixAddrTGSMList) {
-	BasicBlock *BB = I->getParent();
-	fixAddrTGSMFuncSet.insert(BB->getParent());
+  for (StoreInst *I : fixAddrTGSMList) {
+    BasicBlock *BB = I->getParent();
+    fixAddrTGSMFuncSet.insert(BB->getParent());
   }
 
   for (auto &F : ValCtx.DxilMod.GetModule()->functions()) {
@@ -2647,11 +2651,12 @@ ValidateTGSMRaceCondition(std::vector<Instruction *> &fixAddrTGSMList,
 
     BasicBlock *Entry = &F.getEntryBlock();
 
-    for (Instruction *I : fixAddrTGSMList) {
-      BasicBlock *BB = I->getParent();
+    for (StoreInst *SI : fixAddrTGSMList) {
+      BasicBlock *BB = SI->getParent();
       if (BB->getParent() == &F) {
         if (PDT.dominates(BB, Entry)) {
-          ValCtx.EmitInstrError(I, ValidationRule::InstrTGSMRaceCond);
+          if (IsDivergent(SI->getValueOperand()))
+            ValCtx.EmitInstrError(SI, ValidationRule::InstrTGSMRaceCond);
         }
       }
     }
@@ -2662,7 +2667,7 @@ static void ValidateGlobalVariables(ValidationContext &ValCtx) {
   DxilModule &M = ValCtx.DxilMod;
 
   unsigned TGSMSize = 0;
-  std::vector<Instruction*> fixAddrTGSMList;
+  std::vector<StoreInst*> fixAddrTGSMList;
   const DataLayout &DL = M.GetModule()->getDataLayout();
   for (GlobalVariable &GV : M.GetModule()->globals()) {
     ValidateGlobalVariable(GV, ValCtx);

+ 1 - 1
tools/clang/test/CodeGenHLSL/RaceCond.hlsl

@@ -9,7 +9,7 @@ groupshared int sharedData;
 [ numthreads( 64, 2, 2 ) ]
 void main( uint GI : SV_GroupIndex)
 {
-sharedData = 0;
+sharedData = GI;
 InterlockedAdd(sharedData, g_Intensities[GI]);
 g_Intensities[GI] = sharedData;
 }

+ 16 - 0
tools/clang/test/CodeGenHLSL/RaceCond2.hlsl

@@ -0,0 +1,16 @@
+// RUN: %dxc -E main  -T cs_6_0 %s
+
+
+RWBuffer< int > g_Intensities : register(u1);
+
+groupshared int sharedData;
+
+int a;
+int b[2];
+[ numthreads( 64, 2, 2 ) ]
+void main( uint GI : SV_GroupIndex)
+{
+sharedData = b[0];
+InterlockedAdd(sharedData, g_Intensities[GI]);
+g_Intensities[GI] = sharedData;
+}

+ 5 - 0
tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -435,6 +435,7 @@ public:
   TEST_METHOD(CodeGenPrecise4)
   TEST_METHOD(CodeGenPreciseOnCall)
   TEST_METHOD(CodeGenPreciseOnCallNot)
+  TEST_METHOD(CodeGenRaceCond2)
   TEST_METHOD(CodeGenRaw_Buf1)
   TEST_METHOD(CodeGenRcp1)
   TEST_METHOD(CodeGenReadFromOutput)
@@ -2378,6 +2379,10 @@ TEST_F(CompilerTest, CodeGenPreciseOnCallNot) {
   CodeGenTestCheck(L"..\\CodeGenHLSL\\precise_call_not.hlsl");
 }
 
+TEST_F(CompilerTest, CodeGenRaceCond2) {
+  CodeGenTest(L"..\\CodeGenHLSL\\RaceCond2.hlsl");
+}
+
 TEST_F(CompilerTest, CodeGenRaw_Buf1) {
   CodeGenTest(L"..\\CodeGenHLSL\\raw_buf1.hlsl");
 }

+ 16 - 13
tools/clang/unittests/HLSL/ValidationTest.cpp

@@ -125,8 +125,9 @@ public:
   TEST_METHOD(I8Type)
   TEST_METHOD(EmptyStructInBuffer)
   TEST_METHOD(BigStructInBuffer)
-  TEST_METHOD(TGSMRaceCond)
-  TEST_METHOD(TGSMRaceCond2)
+  // TODO: enable this.
+  //TEST_METHOD(TGSMRaceCond)
+  //TEST_METHOD(TGSMRaceCond2)
   TEST_METHOD(AddUint64Odd)
 
   TEST_METHOD(ClipCullMaxComponents)
@@ -1320,17 +1321,19 @@ TEST_F(ValidationTest, BigStructInBuffer) {
   TestCheck(L"..\\CodeGenHLSL\\BigStructInBuffer.hlsl");
 }
 
-TEST_F(ValidationTest, TGSMRaceCond) {
-  TestCheck(L"..\\CodeGenHLSL\\RaceCond.hlsl");
-}
-
-TEST_F(ValidationTest, TGSMRaceCond2) {
-    RewriteAssemblyCheckMsg(L"..\\CodeGenHLSL\\structInBuffer.hlsl", "cs_6_0",
-        "ret void",
-        "store i32 0, i32 addrspace(3)* @\"\\01?sharedData@@3UFoo@@A.3\", align 4\n"
-        "ret void",
-        "Race condition writing to shared memory detected, consider making this write conditional");
-}
+// TODO: enable this.
+//TEST_F(ValidationTest, TGSMRaceCond) {
+//  TestCheck(L"..\\CodeGenHLSL\\RaceCond.hlsl");
+//}
+//
+//TEST_F(ValidationTest, TGSMRaceCond2) {
+//    RewriteAssemblyCheckMsg(L"..\\CodeGenHLSL\\structInBuffer.hlsl", "cs_6_0",
+//        "ret void",
+//        "%TID = call i32 @dx.op.flattenedThreadIdInGroup.i32(i32 96)\n"
+//        "store i32 %TID, i32 addrspace(3)* @\"\\01?sharedData@@3UFoo@@A.3\", align 4\n"
+//        "ret void",
+//        "Race condition writing to shared memory detected, consider making this write conditional");
+//}
 
 TEST_F(ValidationTest, AddUint64Odd) {
   TestCheck(L"..\\CodeGenHLSL\\AddUint64Odd.hlsl");