Corrections to logic for coalescing scalar immediate moves

This PR adds corrections to the logic for coalescing scalar immediate
moves. The first correction addresses logic bug obstructing opportunities for
coalescing movs with identical immediate values and
second correction adds an additional condition to check if the src
operand type is not qword.

Author: anikaushik

IGC/common/igc_flags.h

@@ -117,7 +117,7 @@ DECLARE_IGC_REGKEY(bool, UseLinearScanRA,               false,  "use Linear Scan
 DECLARE_IGC_REGKEY(bool, DisableWriteCombine, false, "Disable write combine. PVC+ only", false)
 DECLARE_IGC_REGKEY(bool, Force32bitConstantGEPLowering, false, "Go back to old version of GEP lowering for constant address space. PVC only", false)
 DECLARE_IGC_REGKEY(bool, NewSpillCostFunction,          false, "Use new spill cost function in VISA RA", false)
-DECLARE_IGC_REGKEY(bool, EnableCoalesceScalarMoves, false, "Enable scalar moves to be coalesced into fewer moves", false)
+DECLARE_IGC_REGKEY(bool, EnableCoalesceScalarMoves, true, "Enable scalar moves to be coalesced into fewer moves", false)
 
 DECLARE_IGC_GROUP("IGC Optimization")
 DECLARE_IGC_REGKEY(bool, AllowMem2Reg,                  false, "Setting this to true makes IGC run mem2reg even when optimizations are disabled", true)

visa/Passes/MergeScalars.cpp

@@ -242,16 +242,22 @@ bool BUNDLE_INFO::doMerge(IR_Builder &builder,
       // lambda to compute the data type based on how many values we
       // packed into a single value
       auto dataTypeSize = [&](G4_Operand *dst) {
+        // size is a power of 2; size is round down to the lowest power of 2
+        // before performing the merge; for example, a size of 3 is round down
+        // to size of 2.
+        // size is capped to sizeLimit, which is 4
         uint32_t totalBytes = size * dst->getTypeSize();
         if (totalBytes < 4)
-          return std::make_tuple(Type_UW, 2);
+          return Type_UW;
         if (totalBytes == 4)
-          return std::make_tuple(Type_UD, 4);
+          return Type_UD;
         if (totalBytes > 4 && totalBytes <= 8)
-          return std::make_tuple(Type_UQ, 8);
-        else
-          MUST_BE_TRUE(false, "invalid data type size");
-        return std::make_tuple(Type_UNDEF, 0);
+          // check if the platform supports 64bit moves
+          if (builder.noInt64()) return Type_UNDEF;
+          return Type_UQ;
+        // otherwise, the size is bigger than available data types
+        // hence, return undefined type
+        return Type_UNDEF;
       };
 
       // In canMergeSource, we check only on whether the instructions
@@ -263,16 +269,21 @@ bool BUNDLE_INFO::doMerge(IR_Builder &builder,
       // 3. check if mov execution mask is no mask SIMD1
       // 4. check if srcs in bundle are immediate values (handled in
       //    canMergeSrc)
-
-      if (!inst[0]->isMov())
-        return false;
-      for (int j = 1; j < size; j++) {
-        if (!inst[j]->isMov() || inst[j - 1]->getDst()->getTopDcl() !=
+      // 5. check if destination type is not float (soft constraint, must address)
+      // 6. check is the total size of packed data type is less than equal to
+      //    largest datatype size (qword).
+      for (int j = 0; j < size; j++) {
+        if (!inst[j]->isMov()) return false;
+        if (!IS_TYPE_INT(inst[j]->getDst()->getType())) return false;
+        if (j > 0 && inst[j - 1]->getDst()->getTopDcl() !=
                                      inst[j]->getDst()->getTopDcl()) {
           return false;
         }
       }
 
+      auto packedType = dataTypeSize(newInst->getDst());
+      if (packedType == Type_UNDEF) return false;
+
       // create the packed value
       // since we also create packed values with non-motonic subregs, the
       // shift amount is subregID * type size (bytes) * 8
@@ -300,9 +311,8 @@ bool BUNDLE_INFO::doMerge(IR_Builder &builder,
         packedVal += ((uint64_t)val << shiftVal);
       }
 
-      auto packedTypeSize = dataTypeSize(newInst->getDst());
-      G4_Type packedType = std::get<0>(packedTypeSize);
-      unsigned packedSize = std::get<1>(packedTypeSize);
+
+      unsigned packedSize = G4_Type_Table[packedType].byteSize;
       // check alignment
       // if destination alignment is less than the datatype of packed value, we
       // cannot do the coalescing
@@ -315,6 +325,7 @@ bool BUNDLE_INFO::doMerge(IR_Builder &builder,
 
       // create a packed type dcl
       G4_Declare *newDcl = builder.createTempVar(0, packedType, Any, "Packed");
+      newDcl->copyAlign(newInst->getDst()->getTopDcl());
       // set the newDcl dcl alias to the instruction destination dcl
       newDcl->setAliasDeclare(newInst->getDst()->getTopDcl(),
                               newInst->getDst()->getSubRegOff());
@@ -614,14 +625,20 @@ bool BUNDLE_INFO::canMergeSource(G4_Operand *src, int srcPos,
       // no coalescing of immediate values possible
       return false;
     } else {
-      if (builder.getOption(vISA_CoalesceScalarMoves)) {
-        if (dstPattern == OPND_PATTERN::CONTIGUOUS) {
+      // coalescing can only be done for data types less than Q/UQ
+      // this is because we are coalescing several moves into a single mov where
+      // the src data type is widened; cannot widen beyond Q/UQ
+      if (builder.getOption(vISA_CoalesceScalarMoves) &&
+              !IS_QTYPE(src->getType())) {
+        if (prevSrc->asImm()->getImm() == src->asImm()->getImm()) {
+          // if the values are the same, then favor opportunities for wider SIMD
+          // moves to broadcast constant value
+          srcPattern[srcPos] = OPND_PATTERN::IDENTICAL;
+        } else if (dstPattern == OPND_PATTERN::CONTIGUOUS) {
           // writing immediate values to different subregs of same
           // GRF
           srcPattern[srcPos] = OPND_PATTERN::PACKED;
         } else {
-          // destination pattern is something other than contiguous
-          // we cannot do packing in this case
           return false;
         }
       } else if (prevSrc->asImm()->getImm() == src->asImm()->getImm()) {