Skip to content

Commit 3b785b6

Browse files
pratikasharigcbot
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.
1 parent 4b8b7cc commit 3b785b6

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
@@ -3448,6 +3448,29 @@ bool Optimizer::foldCmpToCondMod(G4_BB *bb, INST_LIST_ITER &iter) {
34483448
if (getUnsignedTy(T1) != getUnsignedTy(T2)) {
34493449
return false;
34503450
}
3451+
if (!isSupportedCondModForLogicOp && T1 != T2) {
3452+
// If dst signedness of inst is not same as cmp src0, then only z/nz
3453+
// conditions can be evaluated correctly.
3454+
//
3455+
// If inst is a type-conversion mov then it's incorrect to use cmp src0
3456+
// type as mov dst type.
3457+
//
3458+
// mov A:d B:f // f->d mov
3459+
// cmp.gt P1 A:ud 0x0
3460+
//
3461+
// When folding cmp in the mov, we must preserve mov's dst type :d.
3462+
// Otherwise type-conversion semantics change which can lead to wrong
3463+
// result if f->d yields negative result.
3464+
//
3465+
// But if cmp used cmp.z/nz then folding is legal.
3466+
bool isDstSigned = IS_SIGNED_INT(T1);
3467+
bool isDstUnsigned = IS_SIGNED_INT(T1);
3468+
bool isSrcSigned = IS_SIGNED_INT(T2);
3469+
bool isSrcUnsigned = IS_SIGNED_INT(T2);
3470+
if (!(isDstSigned && isSrcSigned) && !(isDstUnsigned && isSrcUnsigned))
3471+
return false;
3472+
}
3473+
34513474
// Skip if the dst needs saturating but it's used as different sign.
34523475
if (inst->getSaturate() && T1 != T2) {
34533476
return false;

0 commit comments

Comments
 (0)