From 9d0f4619fb7f97b1321945fea06bdc616deb84f9 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 3 May 2023 08:56:48 -0700 Subject: [PATCH 1/2] Fix AreFlagsSetToZeroCmp to not consider unsupported formats --- src/coreclr/jit/emit.h | 19 +++++++++++++++++++ src/coreclr/jit/emitxarch.cpp | 15 ++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index 8dfb47932f844..201c0c3d30d24 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -1329,6 +1329,25 @@ class emitter IS_INFO isInfo = emitGetSchedInfo(idInsFmt()); return (isInfo & (IS_AM_RW | IS_AM_WR)) != 0; } + + bool idHasMem() const + { + return idHasMemGen() + || idHasMemStk() + || idHasMemAdr(); + } + bool idHasMemRead() const + { + return idHasMemGenRead() + || idHasMemStkRead() + || idHasMemAdrRead(); + } + bool idHasMemWrite() const + { + return idHasMemGenWrite() + || idHasMemStkWrite() + || idHasMemAdrWrite(); + } #endif // defined(TARGET_XARCH) #ifdef TARGET_ARMARCH insOpts idInsOpt() const diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 867af802c6920..cf5cfc81ced23 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -887,21 +887,30 @@ bool emitter::AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, GenCondition return false; } - // Only consider if safe - // if (!emitCanPeepholeLastIns()) { + // Don't consider if not safe return false; } instrDesc* id = emitLastIns; instruction lastIns = id->idIns(); - if (!id->idHasReg1() || (id->idReg1() != reg)) + if (!id->idIsReg1Write() || (id->idReg1() != reg)) + { + // Don't consider instructions which didn't write a register + return false; + } + + if (id->idHasMemWrite() || id->idIsReg2Write()) { + // Don't consider instructions which also wrote a mem location or second register return false; } + assert(!id->idIsReg3Write()); + assert(!id->idIsReg4Write()); + // Certain instruction like and, or and xor modifies exactly same flags // as "test" instruction. // They reset OF and CF to 0 and modifies SF, ZF and PF. From 0769673b1f448d43e8c1f1ca060033a38beb21ae Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 3 May 2023 09:14:19 -0700 Subject: [PATCH 2/2] Also fix AreFlagsSetForSignJumpOpt --- src/coreclr/jit/emit.h | 12 +++--------- src/coreclr/jit/emitxarch.cpp | 9 ++++++++- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index 201c0c3d30d24..1d49eb6907006 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -1332,21 +1332,15 @@ class emitter bool idHasMem() const { - return idHasMemGen() - || idHasMemStk() - || idHasMemAdr(); + return idHasMemGen() || idHasMemStk() || idHasMemAdr(); } bool idHasMemRead() const { - return idHasMemGenRead() - || idHasMemStkRead() - || idHasMemAdrRead(); + return idHasMemGenRead() || idHasMemStkRead() || idHasMemAdrRead(); } bool idHasMemWrite() const { - return idHasMemGenWrite() - || idHasMemStkWrite() - || idHasMemAdrWrite(); + return idHasMemGenWrite() || idHasMemStkWrite() || idHasMemAdrWrite(); } #endif // defined(TARGET_XARCH) #ifdef TARGET_ARMARCH diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index cf5cfc81ced23..83cdb1c6fe72b 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -965,8 +965,15 @@ bool emitter::AreFlagsSetForSignJumpOpt(regNumber reg, emitAttr opSize, GenCondi instruction lastIns = id->idIns(); insFormat fmt = id->idInsFmt(); - if (!id->idHasReg1() || (id->idReg1() != reg)) + if (!id->idIsReg1Write() || (id->idReg1() != reg)) + { + // Don't consider instructions which didn't write a register + return false; + } + + if (id->idHasMemWrite() || id->idIsReg2Write()) { + // Don't consider instructions which also wrote a mem location or second register return false; }