[Autobackout][FuncReg]Revert of change: 765ac0f

fangliu2020 · igcbot · commit 9731db2353af · 2021-03-28T11:45:11.000+02:00
Enable expandMulPostSchedule pass by default
diff --git a/visa/HWConformity.cpp b/visa/HWConformity.cpp
@@ -2048,10 +2048,10 @@ void HWConformity::doGenerateMacl(INST_LIST_ITER it, G4_BB* bb)
 
         //need extra move for dst
         if (!IS_DTYPE(origDst->getType()) || origDst->getHorzStride() != 1 ||
-            !builder.isOpndAligned(origDst, getGRFSize()))
+            !builder.isOpndAligned(origDst, 32))
         {
             // macl dst must be grf-aligned, packed D/UD as it is also used for the implicit acc source's region
-            G4_DstRegRegion* tmpDst = insertMovAfter(it, origDst, tmpType, bb, GRFALIGN);
+            G4_DstRegRegion* tmpDst = insertMovAfter(it, origDst, tmpType, bb);
             mulInst->setDest(tmpDst);
         }
     }
@@ -2090,10 +2090,10 @@ void HWConformity::doGenerateMacl(INST_LIST_ITER it, G4_BB* bb)
         machIter = bb->insertBefore(++machIter, maclInst);
 
         if (!IS_DTYPE(origDst->getType()) || origDst->getHorzStride() != 1 ||
-            !builder.isOpndAligned(origDst, getGRFSize()))
+            !builder.isOpndAligned(origDst, 32))
         {
             // macl dst must be grf-aligned, packed D/UD as it is also used for the implicit acc source's region
-            G4_DstRegRegion* tmpDst = insertMovAfter(machIter, origDst, tmpType, bb, GRFALIGN);
+            G4_DstRegRegion* tmpDst = insertMovAfter(machIter, origDst, tmpType, bb);
             maclInst->setDest(tmpDst);
         }
     }
@@ -2460,9 +2460,10 @@ bool HWConformity::fixMULInst(INST_LIST_ITER& i, G4_BB* bb)
 // Translate MULH into
 // MUL acc src0 src1
 // MACH dst src0 src1
-void HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
+bool HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
 {
     G4_INST* inst = *i;
+    INST_LIST_ITER iter = i;
     G4_ExecSize execSize = inst->getExecSize();
 
     int inst_opt = inst->getOption();
@@ -2529,20 +2530,23 @@ void HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
             execSize > 1 ? builder.getRegionStride2() : builder.getRegionScalar(),
             dst->getType());
 
+        ++iter;
+
         G4_INST* tmpMov = builder.createMov(execSize, dst, tmpSrc, inst->getOption(), false);
         tmpMov->setPredicate(builder.duplicateOperand(inst->getPredicate()));
 
-        bb->insertAfter(i, tmpMov);
+        bb->insertBefore(iter, tmpMov);
+        //it will decrement back to mov
+        i = iter;
 
-        // Check the new inserted mov inst
-        i++;
-
-        // Need to remove dst from uses list of mulh, and add them to movInst useList
-        // add movInst to uselist of mulh.
-        // Add mulh to def instruction list of movInst
+        /*
+            Need to remove dst from uses list of mulh, and add them to movInst useList
+            add movInst to uselist of mulh.
+            Add mulh to def instruction list of movInst
+        */
         inst->transferUse(tmpMov);
         inst->addDefUse(tmpMov, Opnd_src0);
-        return;
+        return true;
     }
 
     // src1 does not support modifier
@@ -2571,6 +2575,8 @@ void HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
         // Here just create tmp variables to fix srcMod, cond modifier, saturate, etc. And Mul->Mul + Macl expanding will
         // be done in expandMulPostSchedule pass.
 
+        bool newInstInserted = false;
+
         // sat cannot be used at all in the macro sequence
         // this effectivly means sat is broken for mul D D D
         inst->setSaturate(g4::NOSAT);
@@ -2589,30 +2595,32 @@ void HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
         }
 
         INST_LIST_ITER end_iter = i;
-        // this mul will be expanded into mul+macl in expandMulPostSchedule pass. Since expanded macl
-        // must be grf-aligned, so need to make mul to be grf-aligned.
+        // check if the ACC source is aligned to mach dst
+        // ToDo: this should be checked by fixAcc?
         G4_DstRegRegion* dst = inst->getDst();
         if (inst->getSaturate() ||
             dst->getExecTypeSize() > TypeSize(Type_D) ||
-            isPreAssignedRegOffsetNonZero<G4_DstRegRegion>(dst) ||
-            !builder.isOpndAligned(dst, getGRFSize()))
+            isPreAssignedRegOffsetNonZero<G4_DstRegRegion>(dst))
         {
             // add a tmp mov
-            inst->setDest(insertMovAfter(i, dst, dst->getType(), bb, GRFALIGN));
+            inst->setDest(insertMovAfter(i, dst, dst->getType(), bb));
             end_iter++;
+            newInstInserted = true;
         }
 
         if (execSize > builder.getNativeExecSize())
         {
             auto start_iter = i;
-            splitDWMULInst(start_iter, end_iter, bb);
-            // start_iter points to the first half of mulh. Need double check this new inserted mulh to see if need split again
-            i = start_iter;
+            splitDWMULInst(i, end_iter, bb);
+            newInstInserted = true;
         }
-        else
+
+        if (newInstInserted)
         {
+            // it will decrease back to mulh
             i++;
         }
+        return newInstInserted;
     }
     else
     {
@@ -2626,7 +2634,7 @@ void HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
         G4_INST* newMul = builder.createBinOp(G4_mul, execSize,
             acc_dst_opnd, builder.duplicateOperand(src0), builder.duplicateOperand(src1), inst_opt, false);
 
-        bb->insertBefore(i, newMul);
+        bb->insertBefore(iter, newMul);
         inst->copyDefsTo(newMul, false);
 
         fixMulSrc1(std::prev(i), bb);
@@ -2673,16 +2681,10 @@ void HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
         {
             auto start_iter = std::prev(i);
             splitDWMULInst(start_iter, end_iter, bb);
-            // start_iter ponits to the first half of mul. Need to check the new inserted mul/mach instructions
-            i = start_iter;
-        }
-        else
-        {
-            // i points to mach, and need to check the new inserted mul before mach
-            i = std::prev(i);
+            i = end_iter;
         }
+        return true;
     }
-    return;
 }
 
 //
@@ -3567,11 +3569,6 @@ void HWConformity::splitDWMULInst(INST_LIST_ITER& start, INST_LIST_ITER& end, G4
         evenlySplitInst(iter, bb);
         G4_INST* expand_sec_half_op = *iter;
         bb->insertBefore(last_iter, expand_sec_half_op);
-        // For the case that only one instruction needed to split, that is to say start equals to end
-        if (start == end)
-        {
-            start--;
-        }
         end--;
         bb->erase(iter);
     }
@@ -5274,9 +5271,14 @@ void HWConformity::conformBB(G4_BB* bb)
 
         if (inst->opcode() == G4_mulh)
         {
-            fixMULHInst(i, bb);
-            next_iter = i;
-            continue;
+            if (fixMULHInst(i, bb))
+            {
+                // inserted mul before
+                // check the newly added MUL inst
+                i--;
+                next_iter = i;
+                continue;
+            }
         }
 
 #ifdef _DEBUG
@@ -7170,4 +7172,4 @@ void HWConformity::fixSrc1Region(INST_LIST_ITER it, G4_BB* bb)
         G4_Operand* new_src1 = insertMovBefore(it, 1, src1->getType(), bb);
         inst->setSrc(new_src1, 1);
     }
-}
+}
diff --git a/visa/HWConformity.h b/visa/HWConformity.h
@@ -109,7 +109,7 @@ namespace vISA
         void fixPackedSource(INST_LIST_ITER it, G4_BB *bb, G4_Type extype);
         bool fixMathInst(INST_LIST_ITER it, G4_BB *bb);
         bool fixMULInst(INST_LIST_ITER &it, G4_BB *bb);
-        void fixMULHInst(INST_LIST_ITER &i, G4_BB *bb);
+        bool fixMULHInst(INST_LIST_ITER &i, G4_BB *bb);
         void fixMulSrc1(INST_LIST_ITER i, G4_BB* bb);
         void splitDWMULInst(INST_LIST_ITER &start, INST_LIST_ITER &end, G4_BB *bb);
         void fixOpnds(INST_LIST_ITER it, G4_BB *bb, G4_Type& exType);
diff --git a/visa/Optimizer.cpp b/visa/Optimizer.cpp
@@ -11066,7 +11066,6 @@ void Optimizer::expandMulPostSchedule()
             }
             else
             {
-                // create a mach inst
                 inst->setOpcode(G4_mul);
                 maclOrMachInst = builder.createMach(inst->getExecSize(),
                     dst, builder.duplicateOperand(src0), builder.duplicateOperand(src1), origOptions, accType);
@@ -11081,9 +11080,9 @@ void Optimizer::expandMulPostSchedule()
             auto maclOrMachInstIt = bb->insertAfter(it, maclOrMachInst);
 
             // Always add a dummy mov after mach/macl for HW read suppresion W/A
-            auto dummyMovSrc = builder.createSrc(dst->getBase(),
+            auto dummyMovSrc = builder.createSrc(dst->getTopDcl()->getRegVar(),
                 0, 0, builder.getRegionScalar(), Type_D);
-            G4_INST* dummyMov = builder.createMov(g4::SIMD1, builder.createNullDst(Type_D),
+            G4_INST* dummyMov = builder.createMov(g4::SIMD16, builder.createNullDst(Type_D),
                 dummyMovSrc, InstOpt_WriteEnable, false);
             bb->insertAfter(maclOrMachInstIt, dummyMov);
         }
diff --git a/visa/include/VISAOptions.def b/visa/include/VISAOptions.def
@@ -250,7 +250,7 @@ DEF_VISA_OPTION(vISA_DstSrcOverlapWA,               ET_BOOL,  "-dstSrcOverlapWA"
 DEF_VISA_OPTION(vISA_noSendSrcDstOverlap,           ET_BOOL,  "-noSendSrcDstOverlap", UNUSED, false)
 DEF_VISA_OPTION(vISA_cloneSampleInst,               ET_BOOL,  "-cloneSampleInst", UNUSED, false)
 DEF_VISA_OPTION(vISA_cloneEvaluateSampleInst,       ET_BOOL,  "-cloneEvaluateSampleInst", UNUSED, false)
-DEF_VISA_OPTION(vISA_expandMulPostSchedule,         ET_BOOL,  "-expandMulPostSchedule", UNUSED, true)
+DEF_VISA_OPTION(vISA_expandMulPostSchedule,         ET_BOOL,  "-expandMulPostSchedule", UNUSED, false)
 
 //=== HW debugging options ===
 DEF_VISA_OPTION(vISA_GenerateDebugInfo,   ET_BOOL,  "-generateDebugInfo", UNUSED, false)

Original file line number	Diff line number	Diff line change
`@@ -11066,7 +11066,6 @@ void Optimizer::expandMulPostSchedule()`
`11066`	`11066`	`}`
`11067`	`11067`	`else`
`11068`	`11068`	`{`
`11069`		`- // create a mach inst`
`11070`	`11069`	`inst->setOpcode(G4_mul);`
`11071`	`11070`	`maclOrMachInst = builder.createMach(inst->getExecSize(),`
`11072`	`11071`	`dst, builder.duplicateOperand(src0), builder.duplicateOperand(src1), origOptions, accType);`
`@@ -11081,9 +11080,9 @@ void Optimizer::expandMulPostSchedule()`
`11081`	`11080`	`auto maclOrMachInstIt = bb->insertAfter(it, maclOrMachInst);`
`11082`	`11081`
`11083`	`11082`	`// Always add a dummy mov after mach/macl for HW read suppresion W/A`
`11084`		`- auto dummyMovSrc = builder.createSrc(dst->getBase(),`
	`11083`	`+ auto dummyMovSrc = builder.createSrc(dst->getTopDcl()->getRegVar(),`
`11085`	`11084`	`0, 0, builder.getRegionScalar(), Type_D);`
`11086`		`- G4_INST* dummyMov = builder.createMov(g4::SIMD1, builder.createNullDst(Type_D),`
	`11085`	`+ G4_INST* dummyMov = builder.createMov(g4::SIMD16, builder.createNullDst(Type_D),`
`11087`	`11086`	`dummyMovSrc, InstOpt_WriteEnable, false);`
`11088`	`11087`	`bb->insertAfter(maclOrMachInstIt, dummyMov);`
`11089`	`11088`	`}`