Browse Source

Make sure one instruction only pushed into deadInsts once when SimplifyBitCast. (#262)

Xiang Li 8 years ago
parent
commit
7ab7c33f84

+ 10 - 4
tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -3446,15 +3446,21 @@ static void SimplifyBitCast(BitCastOperator *BC, std::vector<Instruction *> &dea
 
 
   for (User *U : BC->users()) {
   for (User *U : BC->users()) {
     if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
     if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
-      if (SimplifyBitCastLoad(LI, FromTy, ToTy, Ptr))
+      if (SimplifyBitCastLoad(LI, FromTy, ToTy, Ptr)) {
+        LI->dropAllReferences();
         deadInsts.emplace_back(LI);
         deadInsts.emplace_back(LI);
+      }
     } else if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
     } else if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
-      if (SimplifyBitCastStore(SI, FromTy, ToTy, Ptr))
+      if (SimplifyBitCastStore(SI, FromTy, ToTy, Ptr)) {
+        SI->dropAllReferences();
         deadInsts.emplace_back(SI);
         deadInsts.emplace_back(SI);
+      }
     } else if (GEPOperator *GEP = dyn_cast<GEPOperator>(U)) {
     } else if (GEPOperator *GEP = dyn_cast<GEPOperator>(U)) {
       if (SimplifyBitCastGEP(GEP, FromTy, ToTy, Ptr))
       if (SimplifyBitCastGEP(GEP, FromTy, ToTy, Ptr))
-        if (Instruction *I = dyn_cast<Instruction>(GEP))
+        if (Instruction *I = dyn_cast<Instruction>(GEP)) {
+          I->dropAllReferences();
           deadInsts.emplace_back(I);
           deadInsts.emplace_back(I);
+        }
     } else if (CallInst *CI = dyn_cast<CallInst>(U)) {
     } else if (CallInst *CI = dyn_cast<CallInst>(U)) {
       // Skip function call.
       // Skip function call.
     } else {
     } else {
@@ -3674,7 +3680,7 @@ static void SimpleTransformForHLDXIR(Instruction *I,
     DXASSERT(!HLMatrixLower::IsMatrixType(ldInst->getType()),
     DXASSERT(!HLMatrixLower::IsMatrixType(ldInst->getType()),
                       "matrix load should use HL LdStMatrix");
                       "matrix load should use HL LdStMatrix");
     Value *Ptr = ldInst->getPointerOperand();
     Value *Ptr = ldInst->getPointerOperand();
-    if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Ptr)) {
+    if (ConstantExpr *CE = dyn_cast_or_null<ConstantExpr>(Ptr)) {
       if (BitCastOperator *BCO = dyn_cast<BitCastOperator>(CE)) {
       if (BitCastOperator *BCO = dyn_cast<BitCastOperator>(CE)) {
         SimplifyBitCast(BCO, deadInsts);
         SimplifyBitCast(BCO, deadInsts);
       }
       }

+ 15 - 0
tools/clang/test/CodeGenHLSL/vec_bitcast.hlsl

@@ -0,0 +1,15 @@
+// RUN: %dxc -E main -T ps_6_0 %s
+
+// f will be casted to vector<float,1> for f.x.
+// And because f is global, so bitcast on f is Constant and shared between the 2 f.x.
+// SimplifyBitCast will run on both load of f.x if we don't take care it.
+
+static float f;
+
+float4 main() : SV_Target
+{
+  float f1 = f.x;
+
+  float f2 = f.x;
+  return f1 + f2;
+}

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

@@ -588,6 +588,7 @@ public:
   TEST_METHOD(CodeGenUpdateCounter)
   TEST_METHOD(CodeGenUpdateCounter)
   TEST_METHOD(CodeGenUpperCaseRegister1);
   TEST_METHOD(CodeGenUpperCaseRegister1);
   TEST_METHOD(CodeGenVcmp)
   TEST_METHOD(CodeGenVcmp)
+  TEST_METHOD(CodeGenVecBitCast)
   TEST_METHOD(CodeGenVec_Comp_Arg)
   TEST_METHOD(CodeGenVec_Comp_Arg)
   TEST_METHOD(CodeGenVecCmpCond)
   TEST_METHOD(CodeGenVecCmpCond)
   TEST_METHOD(CodeGenVecTrunc)
   TEST_METHOD(CodeGenVecTrunc)
@@ -3253,6 +3254,10 @@ TEST_F(CompilerTest, CodeGenVcmp) {
   CodeGenTestCheck(L"..\\CodeGenHLSL\\vcmp.hlsl");
   CodeGenTestCheck(L"..\\CodeGenHLSL\\vcmp.hlsl");
 }
 }
 
 
+TEST_F(CompilerTest, CodeGenVecBitCast) {
+  CodeGenTest(L"..\\CodeGenHLSL\\vec_bitcast.hlsl");
+}
+
 TEST_F(CompilerTest, CodeGenVec_Comp_Arg){
 TEST_F(CompilerTest, CodeGenVec_Comp_Arg){
   CodeGenTest(L"..\\CodeGenHLSL\\vec_comp_arg.hlsl");
   CodeGenTest(L"..\\CodeGenHLSL\\vec_comp_arg.hlsl");
 }
 }