Make sure cmp is correctly split in vISA.

Change-Id: If1b30c4e1160e7b033645a2efbfe5f8c4a31c8ef

Author: jfuentes

IGC/Compiler/CISACodeGen/CISABuilder.cpp

@@ -560,79 +560,28 @@ namespace IGC
             flagDst = true;
         }
 
-        unsigned numParts = 0;
+        VISA_VectorOpnd* opnd0 = GetSourceOperand(src0, m_encoderState.m_srcOperand[0]);
+        VISA_VectorOpnd* opnd1 = GetSourceOperand(src1, m_encoderState.m_srcOperand[1]);
 
-        // Due to a simulator quirk, we need to split the instruction even if the
-        // dst operand of the compare is null, if it "looks" too large,
-        // that is, if the execution size is 16 and the comparison type
-        // is QW.
-        bool bNeedSplitting = false;
-        if (flagDst && needsSplitting(GetAluExecSize(dst)) &&
-            (src0->GetElemSize() > 4 || src1->GetElemSize() > 4))
+        if (flagDst)
         {
-            bNeedSplitting = true;
-            numParts = 2;
+            V(vKernel->AppendVISAComparisonInst(
+                subOp,
+                GetAluEMask(dst),
+                GetAluExecSize(dst),
+                dst->visaPredVariable,
+                opnd0,
+                opnd1));
         }
-
-        bNeedSplitting = bNeedSplitting ||
-            NeedSplitting(src0, m_encoderState.m_srcOperand[0], numParts, true) ||
-            NeedSplitting(src1, m_encoderState.m_srcOperand[1], numParts, true);
-
-        if (bNeedSplitting)
+        else
         {
-            VISA_EMask_Ctrl execMask = GetAluEMask(dst);
-            VISA_Exec_Size fromExecSize = GetAluExecSize(dst);
-            VISA_Exec_Size toExecSize = SplitExecSize(fromExecSize, numParts);
-
-            for (unsigned thePart = 0; thePart != numParts; ++thePart) {
-                SModifier newSrc0Mod = SplitVariable(fromExecSize, toExecSize, thePart, src0, m_encoderState.m_srcOperand[0], true);
-                SModifier newSrc1Mod = SplitVariable(fromExecSize, toExecSize, thePart, src1, m_encoderState.m_srcOperand[1], true);
-                VISA_VectorOpnd* srcOpnd0 = GetSourceOperand(src0, newSrc0Mod);
-                VISA_VectorOpnd* srcOpnd1 = GetSourceOperand(src1, newSrc1Mod);
-                if (flagDst)
-                {
-                    V(vKernel->AppendVISAComparisonInst(subOp,
-                        SplitEMask(fromExecSize, toExecSize, thePart, execMask),
-                        toExecSize,
-                        dst->visaPredVariable,
-                        srcOpnd0, srcOpnd1));
-                }
-                else
-                {
-                    SModifier newDstMod = SplitVariable(fromExecSize, toExecSize, thePart, dst, m_encoderState.m_dstOperand);
-                    VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dst, newDstMod);
-                    V(vKernel->AppendVISAComparisonInst(subOp,
-                        SplitEMask(fromExecSize, toExecSize, thePart, execMask),
-                        toExecSize,
-                        dstOpnd,
-                        srcOpnd0, srcOpnd1));
-                }
-            }
-        }
-        else {
-            VISA_VectorOpnd* opnd0 = GetSourceOperand(src0, m_encoderState.m_srcOperand[0]);
-            VISA_VectorOpnd* opnd1 = GetSourceOperand(src1, m_encoderState.m_srcOperand[1]);
-
-            if (flagDst)
-            {
-                V(vKernel->AppendVISAComparisonInst(
-                    subOp,
-                    GetAluEMask(dst),
-                    GetAluExecSize(dst),
-                    dst->visaPredVariable,
-                    opnd0,
-                    opnd1));
-            }
-            else
-            {
-                V(vKernel->AppendVISAComparisonInst(
-                    subOp,
-                    GetAluEMask(dst),
-                    GetAluExecSize(dst),
-                    GetDestinationOperand(dst, m_encoderState.m_dstOperand),
-                    opnd0,
-                    opnd1));
-            }
+            V(vKernel->AppendVISAComparisonInst(
+                subOp,
+                GetAluEMask(dst),
+                GetAluExecSize(dst),
+                GetDestinationOperand(dst, m_encoderState.m_dstOperand),
+                opnd0,
+                opnd1));
         }
     }
 

visa/InstSplit.cpp

@@ -117,17 +117,19 @@ INST_LIST_ITER InstSplitPass::splitInstruction(INST_LIST_ITER it)
     // Handle split exceptions
     if (!doSplit)
     {
-        if (ISA_Inst_Table[inst->opcode()].type == ISA_Inst_Compare)
+        if (inst->opcode() == G4_cmp)
+        {
             // Due to a simulator quirk, we need to split cmp instruction even if the
             // dst operand of the compare is null, if it "looks" too large,
             // that is, if the execution size is 16 and the comparison type
             // is QW.
-            if (execSize >= 16 &&
+            if (needSplitByExecSize(execSize) && inst->getDst()->isNullReg() &&
                 (G4_Type_Table[inst->getSrc(0)->getType()].byteSize > 4 ||
-                    G4_Type_Table[inst->getSrc(1)->getType()].byteSize > 4 ))
+                    G4_Type_Table[inst->getSrc(1)->getType()].byteSize > 4))
             {
                 doSplit = true;
             }
+        }
     }
 
     if (!doSplit)
@@ -273,6 +275,11 @@ INST_LIST_ITER InstSplitPass::splitInstruction(INST_LIST_ITER it)
     return newInstIterator;
 }
 
+bool InstSplitPass::needSplitByExecSize(G4_ExecSize execSize) const
+{
+    return execSize == g4::SIMD16;
+}
+
 // Compare regRegion of source operand and destination.
 // We put this in a separate function since compareOperand from G4_DstRegRegion
 // and G4_SrcRegRegion don't handle regions that cross 2 GRFs.

visa/InstSplit.h

@@ -39,6 +39,7 @@ namespace vISA
 
     private:
         INST_LIST_ITER splitInstruction(INST_LIST_ITER it);
+        bool needSplitByExecSize(G4_ExecSize ExecSize) const;
         G4_CmpRelation compareSrcDstRegRegion(G4_DstRegRegion* dstRegRegion, G4_Operand* opnd);
         void computeDstBounds(G4_DstRegRegion* dstRegion, uint32_t& leftBound, uint32_t& rightBound);
         void computeSrcBounds(G4_SrcRegRegion* srcRegion, uint32_t& leftBound, uint32_t& rightBound);

visa/IsaVerification.cpp

@@ -553,9 +553,11 @@ void vISAVerifier::verifyRegion(
             }
 
             // Check if the operand may touch more than 2 GRFs due to bad alignment
-            // So far vISA is able to handle the splitting of: Moves
+            // So far vISA is able to handle the splitting of:
+            // moves, logic and cmp instructions
             if (ISA_Inst_Table[opcode].type != ISA_Inst_Mov &&
-                ISA_Inst_Table[opcode].type != ISA_Inst_Logic)
+                ISA_Inst_Table[opcode].type != ISA_Inst_Logic &&
+                ISA_Inst_Table[opcode].type != ISA_Inst_Compare)
             {
                 REPORT_INSTRUCTION(options, (COMMON_ISA_GRF_REG_SIZE * 2u) > last_region_elt_byte,
                     "CISA operand region access out of 2 GRF boundary (within %d bytes): %d",