[JIT] X86/X64 - Eliminate redundant 'cmp' instructions (dotnet#82750)

* Fixed improper peephole zero-extension removal when cdq/cdqe/cwde instructions are involved

* Update regression test

* Formatting

* Handle cdq differently

* Handle cdq differently

* Handle cdq differently

* Initial commit to eliminate redundant 'cmp' instructions

* Take into account cmpxchg

* Take into account cmpxchg

* Feedback

* Temporarily disable cmp opt if we encounter a mov

* Allow checking for mov

* Allow regardless of targetReg

* Allow regardless of targetReg

* Checking if an instruction resets a flag.

* Remove useless comment

* Minor fix

* Abort are checking cmp

* Some refactoring. Taking into account any instruction that modifies flags.

* Minor cleanup

* Remove function from header

* Quick fix

* Sync

* Formatting

* Only look for 'cmp reg, reg'

* Added comment

* Update src/coreclr/jit/emitxarch.cpp

Co-authored-by: Bruce Forstall <[email protected]>

* Update src/coreclr/jit/emitxarch.cpp

Co-authored-by: Bruce Forstall <[email protected]>

---------

Co-authored-by: Bruce Forstall <[email protected]>

src/coreclr/jit/codegenxarch.cpp

@@ -6978,7 +6978,15 @@ void CodeGen::genCompareInt(GenTree* treeNode)
                 targetType = TYP_BOOL; // just a tip for inst_SETCC that movzx is not needed
             }
         }
-        emit->emitInsBinary(ins, emitTypeSize(type), op1, op2);
+
+        emitAttr size    = emitTypeSize(type);
+        bool     canSkip = compiler->opts.OptimizationEnabled() && (ins == INS_cmp) && !op1->isUsedFromMemory() &&
+                       !op2->isUsedFromMemory() && emit->IsRedundantCmp(size, op1->GetRegNum(), op2->GetRegNum());
+
+        if (!canSkip)
+        {
+            emit->emitInsBinary(ins, size, op1, op2);
+        }
     }
 
     // Are we evaluating this into a register?

src/coreclr/jit/emit.h

@@ -2088,6 +2088,7 @@ class emitter
 
 #ifdef TARGET_XARCH
     bool emitIsInstrWritingToReg(instrDesc* id, regNumber reg);
+    bool emitDoesInsModifyFlags(instruction ins);
 #endif // TARGET_XARCH
 
     /************************************************************************/

src/coreclr/jit/emitxarch.cpp

@@ -595,6 +595,24 @@ bool emitter::AreUpper32BitsSignExtended(regNumber reg)
 }
 #endif // TARGET_64BIT
 
+//------------------------------------------------------------------------
+// emitDoesInsModifyFlags: checks if the given instruction modifies flags
+//
+// Arguments:
+//    ins - instruction of interest
+//
+// Return Value:
+//    true if the instruction modifies flags.
+//    false if it does not.
+//
+bool emitter::emitDoesInsModifyFlags(instruction ins)
+{
+    return (CodeGenInterface::instInfo[ins] &
+            (Resets_OF | Resets_SF | Resets_AF | Resets_PF | Resets_CF | Undefined_OF | Undefined_SF | Undefined_AF |
+             Undefined_PF | Undefined_CF | Undefined_ZF | Writes_OF | Writes_SF | Writes_AF | Writes_PF | Writes_CF |
+             Writes_ZF | Restore_SF_ZF_AF_PF_CF));
+}
+
 //------------------------------------------------------------------------
 // emitIsInstrWritingToReg: checks if the given register is being written to
 //
@@ -794,6 +812,75 @@ bool emitter::emitIsInstrWritingToReg(instrDesc* id, regNumber reg)
     return false;
 }
 
+//------------------------------------------------------------------------
+// IsRedundantCmp: determines if there is a 'cmp' instruction that is redundant with the given inputs
+//
+// Arguments:
+//    size - size of 'cmp'
+//    reg1 - op1 register of 'cmp'
+//    reg2 - op2 register of 'cmp'
+//
+// Return Value:
+//    true if there is a redundant 'cmp'
+//
+bool emitter::IsRedundantCmp(emitAttr size, regNumber reg1, regNumber reg2)
+{
+    // Only allow GPRs.
+    // If not a valid register, then return false.
+    if (!genIsValidIntReg(reg1))
+        return false;
+
+    if (!genIsValidIntReg(reg2))
+        return false;
+
+    // Only consider if safe
+    //
+    if (!emitCanPeepholeLastIns())
+    {
+        return false;
+    }
+
+    bool result = false;
+
+    emitPeepholeIterateLastInstrs([&](instrDesc* id) {
+        instruction ins = id->idIns();
+
+        switch (ins)
+        {
+            case INS_cmp:
+            {
+                // We only care about 'cmp reg, reg'.
+                if (id->idInsFmt() != IF_RRD_RRD)
+                    return PEEPHOLE_ABORT;
+
+                if ((id->idReg1() == reg1) && (id->idReg2() == reg2))
+                {
+                    result = (size == id->idOpSize());
+                }
+
+                return PEEPHOLE_ABORT;
+            }
+
+            default:
+                break;
+        }
+
+        if (emitDoesInsModifyFlags(ins))
+        {
+            return PEEPHOLE_ABORT;
+        }
+
+        if (emitIsInstrWritingToReg(id, reg1) || emitIsInstrWritingToReg(id, reg2))
+        {
+            return PEEPHOLE_ABORT;
+        }
+
+        return PEEPHOLE_CONTINUE;
+    });
+
+    return result;
+}
+
 //------------------------------------------------------------------------
 // AreFlagsSetToZeroCmp: Checks if the previous instruction set the SZ, and optionally OC, flags to
 //                       the same values as if there were a compare to 0

src/coreclr/jit/emitxarch.h

@@ -138,6 +138,8 @@ bool AreUpper32BitsZero(regNumber reg);
 bool AreUpper32BitsSignExtended(regNumber reg);
 #endif // TARGET_64BIT
 
+bool IsRedundantCmp(emitAttr size, regNumber reg1, regNumber reg2);
+
 bool AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, GenCondition cond);
 bool AreFlagsSetForSignJumpOpt(regNumber reg, emitAttr opSize, GenCondition cond);