Skip to content

Commit 9d27898

Browse files
jfuentessys_zuul
authored andcommitted
Remove need split check for mov instructions as the instruction
splitting is now handled by vISA. vISA verifier doesn't check >2 GRF crossing in mov instructions. Change-Id: I098e0ac62624ec3b3254d71f825fa96cfd183ea5
1 parent 6f30415 commit 9d27898

File tree

2 files changed

+98
-173
lines changed

2 files changed

+98
-173
lines changed

IGC/Compiler/CISACodeGen/CISABuilder.cpp

Lines changed: 78 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -1588,188 +1588,109 @@ namespace IGC
15881588
srcAlias = src;
15891589
}
15901590
}
1591-
unsigned numParts = 0;
1592-
if (NeedSplitting(dst, m_encoderState.m_dstOperand, numParts) ||
1593-
NeedSplitting(src, m_encoderState.m_srcOperand[0], numParts, true)) {
15941591

1595-
VISA_EMask_Ctrl execMask = GetAluEMask(dst);
1596-
VISA_Exec_Size fromExecSize = GetAluExecSize(dst);
1597-
VISA_Exec_Size toExecSize = SplitExecSize(fromExecSize, numParts);
1598-
1599-
for (unsigned thePart = 0; thePart != numParts; ++thePart) {
1600-
SModifier newDstMod = SplitVariable(fromExecSize, toExecSize, thePart, dst, m_encoderState.m_dstOperand);
1601-
SModifier newSrcMod = SplitVariable(fromExecSize, toExecSize, thePart, src, m_encoderState.m_srcOperand[0], true);
1602-
if (Need64BitEmu) {
1603-
if (Is64BitSrc && Is64BitDst) {
1604-
// Generate data movement on Lo part.
1605-
SModifier LoDstMod = EmulateVariable(dst, newDstMod, false, false);
1606-
SModifier LoSrcMod = EmulateVariable(src, newSrcMod, false, true);
1607-
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dstAlias, LoDstMod);
1608-
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, LoSrcMod);
1609-
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1610-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1611-
SplitEMask(fromExecSize, toExecSize, thePart, execMask),
1612-
toExecSize,
1613-
dstOpnd, srcOpnd));
1614-
// Generate data movement on Hi part.
1615-
SModifier HiDstMod = EmulateVariable(dst, newDstMod, true, false);
1616-
SModifier HiSrcMod = EmulateVariable(src, newSrcMod, true, true);
1617-
dstOpnd = GetDestinationOperand(dstAlias, HiDstMod);
1618-
srcOpnd = srcImmHi ? srcImmHi : GetSourceOperand(srcAlias, HiSrcMod);
1619-
predOpnd = GetFlagOperand(m_encoderState.m_flag);
1620-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1621-
SplitEMask(fromExecSize, toExecSize, thePart, execMask),
1622-
toExecSize,
1623-
dstOpnd, srcOpnd));
1624-
}
1625-
else if (Is64BitSrc) {
1626-
IGC_ASSERT_MESSAGE(!Is64BitDst, "Expect non 64-bit dst!");
1627-
// Generate data movement on Lo part only.
1628-
SModifier LoSrcMod = EmulateVariable(src, newSrcMod, false, true);
1629-
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dstAlias, newDstMod);
1630-
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, LoSrcMod);
1631-
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1632-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1633-
SplitEMask(fromExecSize, toExecSize, thePart, execMask),
1634-
toExecSize,
1635-
dstOpnd, srcOpnd));
1636-
}
1637-
else {
1638-
IGC_ASSERT_MESSAGE(Is64BitDst, "Expect 64-bit dst!");
1639-
IGC_ASSERT_MESSAGE(!Is64BitSrc, "Expect non 64-bit src!");
1640-
1641-
// Generate data movement on Lo part.
1642-
SModifier LoDstMod = EmulateVariable(dst, newDstMod, false, false);
1643-
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dstAlias, LoDstMod);
1644-
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, newSrcMod);
1645-
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1646-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1647-
SplitEMask(fromExecSize, toExecSize, thePart, execMask),
1648-
toExecSize,
1649-
dstOpnd, srcOpnd));
1650-
// Generate data movement on Hi part.
1651-
unsigned ImmHi = 0U;
1652-
V(vKernel->CreateVISAImmediate(srcImmHi, &ImmHi, ISA_TYPE_UD));
1653-
SModifier HiDstMod = EmulateVariable(dst, newDstMod, true, false);
1654-
dstOpnd = GetDestinationOperand(dstAlias, HiDstMod);
1655-
srcOpnd = srcImmHi;
1656-
predOpnd = GetFlagOperand(m_encoderState.m_flag);
1657-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1658-
SplitEMask(fromExecSize, toExecSize, thePart, execMask),
1659-
toExecSize,
1660-
dstOpnd, srcOpnd));
1661-
}
1662-
}
1663-
else {
1664-
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dst, newDstMod);
1665-
VISA_VectorOpnd* srcOpnd = GetSourceOperand(src, newSrcMod);
1666-
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1667-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1668-
SplitEMask(fromExecSize, toExecSize, thePart, execMask),
1669-
toExecSize,
1670-
dstOpnd, srcOpnd));
1671-
}
1672-
}
1673-
}
1674-
else {
1675-
if (Need64BitEmu) {
1676-
if (Is64BitSrc && Is64BitDst) {
1677-
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1678-
if (!predOpnd && !IsSat() && dst->IsUniform() && src->IsUniform() && !src->IsImmediate())
1679-
{
1680-
// special handling for uniform 64b copy by generating SIMD2 move instead of 2xSIMD1
1681-
// technically we need to check for src modifier and whether dst/src are indirect operand as well,
1682-
// but it doesn't look like the original code below is doing it anyway..
1683-
SModifier dstAsUDMod = m_encoderState.m_dstOperand;
1684-
dstAsUDMod.subReg *= 2;
1685-
SModifier srcAsUDMod = m_encoderState.m_srcOperand[0];
1686-
srcAsUDMod.region[0] = 1;
1687-
srcAsUDMod.region[1] = 1;
1688-
srcAsUDMod.region[2] = 0;
1689-
srcAsUDMod.specialRegion = true;
1690-
srcAsUDMod.subReg *= 2;
1691-
auto dstOpnd = GetDestinationOperand(dstAlias, dstAsUDMod);
1692-
auto SIMDSize = lanesToSIMDMode(numLanes(m_encoderState.m_uniformSIMDSize) * 2);
1693-
auto srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, srcAsUDMod);
1694-
V(vKernel->AppendVISADataMovementInst(opcode, nullptr, false, vISA_EMASK_M1_NM, visaExecSize(SIMDSize),
1695-
dstOpnd, srcOpnd));
1696-
}
1697-
else
1698-
{
1699-
// Generate data movement on Lo part.
1700-
SModifier LoDstMod = EmulateVariable(dst, m_encoderState.m_dstOperand, false, false);
1701-
SModifier LoSrcMod = EmulateVariable(src, m_encoderState.m_srcOperand[0], false, true);
1702-
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dstAlias, LoDstMod);
1703-
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, LoSrcMod);
1704-
1705-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1706-
GetAluEMask(dst),
1707-
GetAluExecSize(dst),
1708-
dstOpnd, srcOpnd));
1709-
// Generate data movement on Hi part.
1710-
SModifier HiDstMod = EmulateVariable(dst, m_encoderState.m_dstOperand, true, false);
1711-
SModifier HiSrcMod = EmulateVariable(src, m_encoderState.m_srcOperand[0], true, true);
1712-
dstOpnd = GetDestinationOperand(dstAlias, HiDstMod);
1713-
srcOpnd = srcImmHi ? srcImmHi : GetSourceOperand(srcAlias, HiSrcMod);
1714-
predOpnd = GetFlagOperand(m_encoderState.m_flag);
1715-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1716-
GetAluEMask(dst),
1717-
GetAluExecSize(dst),
1718-
dstOpnd, srcOpnd));
1719-
}
1720-
}
1721-
else if (Is64BitSrc) {
1722-
IGC_ASSERT_MESSAGE(!Is64BitDst, "Expect non 64-bit dst!");
1723-
// Generate data movement on Lo part only.
1724-
SModifier LoSrcMod = EmulateVariable(src, m_encoderState.m_srcOperand[0], false, true);
1725-
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dstAlias, m_encoderState.m_dstOperand);
1726-
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, LoSrcMod);
1727-
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1728-
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1729-
GetAluEMask(dst),
1730-
GetAluExecSize(dst),
1592+
if (Need64BitEmu)
1593+
{
1594+
if (Is64BitSrc && Is64BitDst)
1595+
{
1596+
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1597+
if (!predOpnd && !IsSat() && dst->IsUniform() && src->IsUniform() && !src->IsImmediate())
1598+
{
1599+
// special handling for uniform 64b copy by generating SIMD2 move instead of 2xSIMD1
1600+
// technically we need to check for src modifier and whether dst/src are indirect operand as well,
1601+
// but it doesn't look like the original code below is doing it anyway..
1602+
SModifier dstAsUDMod = m_encoderState.m_dstOperand;
1603+
dstAsUDMod.subReg *= 2;
1604+
SModifier srcAsUDMod = m_encoderState.m_srcOperand[0];
1605+
srcAsUDMod.region[0] = 1;
1606+
srcAsUDMod.region[1] = 1;
1607+
srcAsUDMod.region[2] = 0;
1608+
srcAsUDMod.specialRegion = true;
1609+
srcAsUDMod.subReg *= 2;
1610+
auto dstOpnd = GetDestinationOperand(dstAlias, dstAsUDMod);
1611+
auto SIMDSize = lanesToSIMDMode(numLanes(m_encoderState.m_uniformSIMDSize) * 2);
1612+
auto srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, srcAsUDMod);
1613+
V(vKernel->AppendVISADataMovementInst(opcode, nullptr, false, vISA_EMASK_M1_NM, visaExecSize(SIMDSize),
17311614
dstOpnd, srcOpnd));
17321615
}
1733-
else {
1734-
IGC_ASSERT_MESSAGE(Is64BitDst, "Expect 64-bit dst!");
1735-
IGC_ASSERT_MESSAGE(!Is64BitSrc, "Expect non 64-bit src");
1736-
1616+
else
1617+
{
17371618
// Generate data movement on Lo part.
17381619
SModifier LoDstMod = EmulateVariable(dst, m_encoderState.m_dstOperand, false, false);
1620+
SModifier LoSrcMod = EmulateVariable(src, m_encoderState.m_srcOperand[0], false, true);
17391621
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dstAlias, LoDstMod);
1740-
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, m_encoderState.m_srcOperand[0]);
1741-
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1622+
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, LoSrcMod);
1623+
17421624
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
17431625
GetAluEMask(dst),
17441626
GetAluExecSize(dst),
17451627
dstOpnd, srcOpnd));
17461628
// Generate data movement on Hi part.
1747-
unsigned ImmHi = 0U;
1748-
V(vKernel->CreateVISAImmediate(srcImmHi, &ImmHi, ISA_TYPE_UD));
17491629
SModifier HiDstMod = EmulateVariable(dst, m_encoderState.m_dstOperand, true, false);
1630+
SModifier HiSrcMod = EmulateVariable(src, m_encoderState.m_srcOperand[0], true, true);
17501631
dstOpnd = GetDestinationOperand(dstAlias, HiDstMod);
1751-
srcOpnd = srcImmHi;
1632+
srcOpnd = srcImmHi ? srcImmHi : GetSourceOperand(srcAlias, HiSrcMod);
17521633
predOpnd = GetFlagOperand(m_encoderState.m_flag);
17531634
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
17541635
GetAluEMask(dst),
17551636
GetAluExecSize(dst),
17561637
dstOpnd, srcOpnd));
17571638
}
17581639
}
1759-
else {
1760-
VISA_VectorOpnd* srcOpnd = GetSourceOperand(src, m_encoderState.m_srcOperand[0]);
1761-
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dst, m_encoderState.m_dstOperand);
1640+
else if (Is64BitSrc)
1641+
{
1642+
IGC_ASSERT_MESSAGE(!Is64BitDst, "Expect non 64-bit dst!");
1643+
// Generate data movement on Lo part only.
1644+
SModifier LoSrcMod = EmulateVariable(src, m_encoderState.m_srcOperand[0], false, true);
1645+
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dstAlias, m_encoderState.m_dstOperand);
1646+
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, LoSrcMod);
17621647
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1763-
V(vKernel->AppendVISADataMovementInst(
1764-
opcode,
1765-
predOpnd,
1766-
IsSat(),
1648+
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
17671649
GetAluEMask(dst),
17681650
GetAluExecSize(dst),
1769-
dstOpnd,
1770-
srcOpnd));
1651+
dstOpnd, srcOpnd));
1652+
}
1653+
else
1654+
{
1655+
IGC_ASSERT_MESSAGE(Is64BitDst, "Expect 64-bit dst!");
1656+
IGC_ASSERT_MESSAGE(!Is64BitSrc, "Expect non 64-bit src");
1657+
1658+
// Generate data movement on Lo part.
1659+
SModifier LoDstMod = EmulateVariable(dst, m_encoderState.m_dstOperand, false, false);
1660+
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dstAlias, LoDstMod);
1661+
VISA_VectorOpnd* srcOpnd = srcImmLo ? srcImmLo : GetSourceOperand(srcAlias, m_encoderState.m_srcOperand[0]);
1662+
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1663+
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1664+
GetAluEMask(dst),
1665+
GetAluExecSize(dst),
1666+
dstOpnd, srcOpnd));
1667+
// Generate data movement on Hi part.
1668+
unsigned ImmHi = 0U;
1669+
V(vKernel->CreateVISAImmediate(srcImmHi, &ImmHi, ISA_TYPE_UD));
1670+
SModifier HiDstMod = EmulateVariable(dst, m_encoderState.m_dstOperand, true, false);
1671+
dstOpnd = GetDestinationOperand(dstAlias, HiDstMod);
1672+
srcOpnd = srcImmHi;
1673+
predOpnd = GetFlagOperand(m_encoderState.m_flag);
1674+
V(vKernel->AppendVISADataMovementInst(opcode, predOpnd, IsSat(),
1675+
GetAluEMask(dst),
1676+
GetAluExecSize(dst),
1677+
dstOpnd, srcOpnd));
17711678
}
17721679
}
1680+
else
1681+
{
1682+
VISA_VectorOpnd* srcOpnd = GetSourceOperand(src, m_encoderState.m_srcOperand[0]);
1683+
VISA_VectorOpnd* dstOpnd = GetDestinationOperand(dst, m_encoderState.m_dstOperand);
1684+
VISA_PredOpnd* predOpnd = GetFlagOperand(m_encoderState.m_flag);
1685+
V(vKernel->AppendVISADataMovementInst(
1686+
opcode,
1687+
predOpnd,
1688+
IsSat(),
1689+
GetAluEMask(dst),
1690+
GetAluExecSize(dst),
1691+
dstOpnd,
1692+
srcOpnd));
1693+
}
17731694
}
17741695
}
17751696

visa/IsaVerification.cpp

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -551,25 +551,29 @@ void vISAVerifier::verifyRegion(
551551
(exec_sz - 1) * h_stride_val * VN_size + VN_size - 1;
552552
}
553553

554-
REPORT_INSTRUCTION(options,(COMMON_ISA_GRF_REG_SIZE * 2u) > last_region_elt_byte,
555-
"CISA operand region access out of 2 GRF boundary (within %d bytes): %d",
556-
(COMMON_ISA_GRF_REG_SIZE * 2),
557-
last_region_elt_byte);
554+
// Check if the operand may touch more than 2 GRFs due to bad alignment
555+
// So far vISA is able to handle the splitting of: Moves
556+
if (ISA_Inst_Mov != ISA_Inst_Table[opcode].type)
557+
{
558+
REPORT_INSTRUCTION(options, (COMMON_ISA_GRF_REG_SIZE * 2u) > last_region_elt_byte,
559+
"CISA operand region access out of 2 GRF boundary (within %d bytes): %d",
560+
(COMMON_ISA_GRF_REG_SIZE * 2),
561+
last_region_elt_byte);
562+
563+
// check if the operand may touch more than 2 GRFs due to bad alignment
564+
unsigned startByte = getStartByteOffset(header, var, numPreDefinedVars) +
565+
row_offset * COMMON_ISA_GRF_REG_SIZE +
566+
col_offset * CISATypeTable[var->getType()].typeSize;
567+
unsigned endByte = startByte + last_region_elt_byte;
568+
unsigned startGRF = startByte / COMMON_ISA_GRF_REG_SIZE;
569+
unsigned endGRF = endByte / COMMON_ISA_GRF_REG_SIZE;
570+
REPORT_INSTRUCTION(options, endGRF == startGRF || endGRF == (startGRF + 1),
571+
"CISA operand accesses more than 2 GRF due to mis-alignment: start byte offset = %d, end byte offset = %d",
572+
startByte, endByte);
573+
}
558574

559575
unsigned firstElementIndex = row_offset * COMMON_ISA_GRF_REG_SIZE + col_offset;
560576

561-
// check if the operand may touch more than 2 GRFs due to bad alignment
562-
unsigned startByte = getStartByteOffset(header, var, numPreDefinedVars) +
563-
row_offset * COMMON_ISA_GRF_REG_SIZE +
564-
col_offset * CISATypeTable[var->getType()].typeSize;
565-
unsigned endByte = startByte + last_region_elt_byte;
566-
unsigned startGRF = startByte / COMMON_ISA_GRF_REG_SIZE;
567-
unsigned endGRF = endByte / COMMON_ISA_GRF_REG_SIZE;
568-
REPORT_INSTRUCTION(options,endGRF == startGRF || endGRF == (startGRF + 1),
569-
"CISA operand accesses more than 2 GRF due to mis-alignment: start byte offset = %d, end byte offset = %d",
570-
startByte, endByte);
571-
572-
573577
for (int i = 0; i < exec_sz / width_val; i++)
574578
{
575579
for (int j = 0; j < width_val; j++)

0 commit comments

Comments
 (0)