Skip to content

Commit 47e7cc7

Browse files
pratikasharpszymich
authored andcommitted
Fix logic when folding cmp in to condition modifier
We can fold condition modifier in def when def and use in cmp use same type. When they use different types with same bit-width but different signedness, folding requires more checks. Consider following pseudo code: mov A:d B:f cmp.ge P A:ud 0x0 We cannot fold above cmp in to mov because A's dst is :d whereas the cmp interprets it as :ud. However, we could still fold it in if cmp condition was .z or .nz as these yield same result irrespective of signedness. (cherry picked from commit 84aeb2a)
1 parent 9fa20a2 commit 47e7cc7

File tree

1 file changed

+23
-0
lines changed

1 file changed

+23
-0
lines changed

visa/Optimizer.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3444,6 +3444,29 @@ bool Optimizer::foldCmpToCondMod(G4_BB *bb, INST_LIST_ITER &iter) {
34443444
if (getUnsignedTy(T1) != getUnsignedTy(T2)) {
34453445
return false;
34463446
}
3447+
if (!isSupportedCondModForLogicOp && T1 != T2) {
3448+
// If dst signedness of inst is not same as cmp src0, then only z/nz
3449+
// conditions can be evaluated correctly.
3450+
//
3451+
// If inst is a type-conversion mov then it's incorrect to use cmp src0
3452+
// type as mov dst type.
3453+
//
3454+
// mov A:d B:f // f->d mov
3455+
// cmp.gt P1 A:ud 0x0
3456+
//
3457+
// When folding cmp in the mov, we must preserve mov's dst type :d.
3458+
// Otherwise type-conversion semantics change which can lead to wrong
3459+
// result if f->d yields negative result.
3460+
//
3461+
// But if cmp used cmp.z/nz then folding is legal.
3462+
bool isDstSigned = IS_SIGNED_INT(T1);
3463+
bool isDstUnsigned = IS_SIGNED_INT(T1);
3464+
bool isSrcSigned = IS_SIGNED_INT(T2);
3465+
bool isSrcUnsigned = IS_SIGNED_INT(T2);
3466+
if (!(isDstSigned && isSrcSigned) && !(isDstUnsigned && isSrcUnsigned))
3467+
return false;
3468+
}
3469+
34473470
// Skip if the dst needs saturating but it's used as different sign.
34483471
if (inst->getSaturate() && T1 != T2) {
34493472
return false;

0 commit comments

Comments
 (0)