Enable expandMulPostSchedule pass by default

Author: fangliu2020

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, 32))
+            !builder.isOpndAligned(origDst, getGRFSize()))
         {
             // 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);
+            G4_DstRegRegion* tmpDst = insertMovAfter(it, origDst, tmpType, bb, GRFALIGN);
             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, 32))
+            !builder.isOpndAligned(origDst, getGRFSize()))
         {
             // 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);
+            G4_DstRegRegion* tmpDst = insertMovAfter(machIter, origDst, tmpType, bb, GRFALIGN);
             maclInst->setDest(tmpDst);
         }
     }
@@ -2460,10 +2460,9 @@ bool HWConformity::fixMULInst(INST_LIST_ITER& i, G4_BB* bb)
 // Translate MULH into
 // MUL acc src0 src1
 // MACH dst src0 src1
-bool HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
+void 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();
@@ -2530,23 +2529,20 @@ bool 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->insertBefore(iter, tmpMov);
-        //it will decrement back to mov
-        i = iter;
+        bb->insertAfter(i, tmpMov);
 
-        /*
-            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
-        */
+        // 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
         inst->transferUse(tmpMov);
         inst->addDefUse(tmpMov, Opnd_src0);
-        return true;
+        return;
     }
 
     // src1 does not support modifier
@@ -2575,8 +2571,6 @@ bool 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);
@@ -2595,32 +2589,30 @@ bool HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
         }
 
         INST_LIST_ITER end_iter = i;
-        // check if the ACC source is aligned to mach dst
-        // ToDo: this should be checked by fixAcc?
+        // 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.
         G4_DstRegRegion* dst = inst->getDst();
         if (inst->getSaturate() ||
             dst->getExecTypeSize() > TypeSize(Type_D) ||
-            isPreAssignedRegOffsetNonZero<G4_DstRegRegion>(dst))
+            isPreAssignedRegOffsetNonZero<G4_DstRegRegion>(dst) ||
+            !builder.isOpndAligned(dst, getGRFSize()))
         {
             // add a tmp mov
-            inst->setDest(insertMovAfter(i, dst, dst->getType(), bb));
+            inst->setDest(insertMovAfter(i, dst, dst->getType(), bb, GRFALIGN));
             end_iter++;
-            newInstInserted = true;
         }
 
         if (execSize > builder.getNativeExecSize())
         {
             auto start_iter = i;
-            splitDWMULInst(i, end_iter, bb);
-            newInstInserted = true;
+            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;
         }
-
-        if (newInstInserted)
+        else
         {
-            // it will decrease back to mulh
             i++;
         }
-        return newInstInserted;
     }
     else
     {
@@ -2634,7 +2626,7 @@ bool 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(iter, newMul);
+        bb->insertBefore(i, newMul);
         inst->copyDefsTo(newMul, false);
 
         fixMulSrc1(std::prev(i), bb);
@@ -2681,10 +2673,16 @@ bool HWConformity::fixMULHInst(INST_LIST_ITER& i, G4_BB* bb)
         {
             auto start_iter = std::prev(i);
             splitDWMULInst(start_iter, end_iter, bb);
-            i = end_iter;
+            // 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);
         }
-        return true;
     }
+    return;
 }
 
 //
@@ -3569,6 +3567,11 @@ 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);
     }
@@ -5271,14 +5274,9 @@ void HWConformity::conformBB(G4_BB* bb)
 
         if (inst->opcode() == G4_mulh)
         {
-            if (fixMULHInst(i, bb))
-            {
-                // inserted mul before
-                // check the newly added MUL inst
-                i--;
-                next_iter = i;
-                continue;
-            }
+            fixMULHInst(i, bb);
+            next_iter = i;
+            continue;
         }
 
 #ifdef _DEBUG
@@ -7172,4 +7170,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);
     }
-}
+}

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);
-        bool fixMULHInst(INST_LIST_ITER &i, G4_BB *bb);
+        void 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);

visa/Optimizer.cpp

@@ -11066,6 +11066,7 @@ 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);
@@ -11080,9 +11081,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->getTopDcl()->getRegVar(),
+            auto dummyMovSrc = builder.createSrc(dst->getBase(),
                 0, 0, builder.getRegionScalar(), Type_D);
-            G4_INST* dummyMov = builder.createMov(g4::SIMD16, builder.createNullDst(Type_D),
+            G4_INST* dummyMov = builder.createMov(g4::SIMD1, builder.createNullDst(Type_D),
                 dummyMovSrc, InstOpt_WriteEnable, false);
             bb->insertAfter(maclOrMachInstIt, dummyMov);
         }

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, false)
+DEF_VISA_OPTION(vISA_expandMulPostSchedule,         ET_BOOL,  "-expandMulPostSchedule", UNUSED, true)
 
 //=== HW debugging options ===
 DEF_VISA_OPTION(vISA_GenerateDebugInfo,   ET_BOOL,  "-generateDebugInfo", UNUSED, false)