Skip to content

Commit 78db6ad

Browse files
jgu222igcbot
authored andcommitted
Fixed incorrect types for rotate inst
As rotate needs to keep its dst/src0 type the same and no widening or narrowing should be done on those operands, make sure copyProg and defHoist do not change the type of dst/src0. Also, make sure to use the larger type of dst/src0 as rotate type to be conservative. This is to handle the case in which imm src0's type has been changed.
1 parent bf2e38d commit 78db6ad

File tree

2 files changed

+36
-4
lines changed

2 files changed

+36
-4
lines changed

visa/G4_IR.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2368,6 +2368,13 @@ bool G4_INST::canPropagateTo(
23682368
}
23692369
}
23702370

2371+
if ((useInst_op == G4_rol || useInst_op == G4_ror) && opndNum == 0 &&
2372+
(TypeSize(dstType) != TypeSize(srcType) || TypeSize(dstType) != TypeSize(useType)))
2373+
{
2374+
// rotation's src0 is sensitive to its type size. No prop if type sizes are different.
2375+
return false;
2376+
}
2377+
23712378
// In general, to check whether that MOV could be propagated:
23722379
//
23732380
// dst/T1 = src/T0;
@@ -2936,6 +2943,13 @@ bool G4_INST::canHoistTo(const G4_INST *defInst, bool simdBB) const
29362943
}
29372944
}
29382945

2946+
if ((defInst->opcode() == G4_rol || defInst->opcode() == G4_ror) &&
2947+
(TypeSize(defDstType) != TypeSize(srcType) || TypeSize(defDstType) != TypeSize(dstType)))
2948+
{
2949+
// rotate's dst is sensitive to its size. Make sure operand's size remains unchanged.
2950+
return false;
2951+
}
2952+
29392953
// Cannot do hoisting if the use inst has src modifier.
29402954
if (getSrc(0)->asSrcRegRegion()->hasModifier())
29412955
{

visa/HWConformity.cpp

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,10 +1684,28 @@ bool HWConformity::fixRotate(INST_LIST_ITER i, G4_BB* bb)
16841684

16851685
if (dst->getTypeSize() != src->getTypeSize())
16861686
{
1687-
// keep exec type same and change dst to be same type as src
1688-
replaceDst(i, src->getType());
1689-
dst = inst->getDst();
1690-
changed = true;
1687+
// Expect rotate has the same size for its src0 and its dst.
1688+
// But visa could change the imm src0 to a different type
1689+
// Use the larger of src0 and dst as rotation type and keep exec type same.
1690+
if (dst->getTypeSize() > src->getTypeSize())
1691+
{
1692+
// use dst type as rotation type
1693+
G4_Operand* newSrc = nullptr;
1694+
if (src->isImm())
1695+
{
1696+
newSrc = builder.createImm(newSrc->asImm()->getImm(), dst->getType());
1697+
}
1698+
else {
1699+
newSrc = insertMovBefore(i, 0, dst->getType(), bb);
1700+
}
1701+
inst->setSrc(newSrc, 0);
1702+
}
1703+
else
1704+
{
1705+
// use src type as rotation type.
1706+
inst->setDest(insertMovAfter(i, dst, src->getType(), bb));
1707+
}
1708+
// let it fall-thru
16911709
}
16921710

16931711
if (dst->getType() == Type_W)

0 commit comments

Comments
 (0)