From 35297f5774809e90ddf8c50ec09b03e988ce34a2 Mon Sep 17 00:00:00 2001 From: Ashay Rane Date: Wed, 13 Aug 2025 01:19:40 -0500 Subject: [PATCH 1/2] Elide redundant test instruction for signed comparison predicates Prior to this patch, when emitting code for x64, if an instruction updated the flags register and if the following instruction was an equals or not-equals comparison with zero, the runtime elided the `test` instruction for materializing the comparison. Although correct, this is limiting, since it excludes the predicates SGT, SGE, SLT, and SLE. This patch extends the optimization to allow the omission of the `test` instruction for all signed integer comparisons. We skip unsigned integer comparisons since unsigned comparisons with zero can be evaluated earlier in the compilation process. Fix #117866 --- src/coreclr/jit/emitxarch.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 30b5ea482838b8..a11a5162560f2d 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -1464,7 +1464,9 @@ bool emitter::AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, GenCondition return id->idOpSize() == opSize; } - if ((cond.GetCode() == GenCondition::NE) || (cond.GetCode() == GenCondition::EQ)) + if ((cond.GetCode() == GenCondition::NE) || (cond.GetCode() == GenCondition::EQ) || + (cond.GetCode() == GenCondition::SLE) || (cond.GetCode() == GenCondition::SLT) || + (cond.GetCode() == GenCondition::SGE) || (cond.GetCode() == GenCondition::SGT)) { if (DoesWriteZeroFlag(lastIns) && IsFlagsAlwaysModified(id)) { From dc10564f65b63532f452c58f305932335ebff0ff Mon Sep 17 00:00:00 2001 From: Ashay Rane Date: Wed, 13 Aug 2025 23:43:02 -0500 Subject: [PATCH 2/2] Check whether preceding instruction updates required flags Add new checks for specific flag bits based on the requested condition predicate --- src/coreclr/jit/emitxarch.cpp | 37 ++++++++++++++++++++++++++++++++--- src/coreclr/jit/emitxarch.h | 1 + 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index a11a5162560f2d..075dffdd5ffdc3 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -709,6 +709,22 @@ bool emitter::HasRegularWideImmediateForm(instruction ins) return (flags & INS_FLAGS_Has_Sbit) != 0; } +//------------------------------------------------------------------------ +// DoesWriteOverflowFlag: check if the instruction write the +// OF flag. +// +// Arguments: +// ins - instruction to test +// +// Return Value: +// true if instruction writes the OF flag, false otherwise. +// +bool emitter::DoesWriteOverflowFlag(instruction ins) +{ + insFlags flags = CodeGenInterface::instInfo[ins]; + return (flags & Writes_OF) != 0; +} + //------------------------------------------------------------------------ // DoesWriteZeroFlag: check if the instruction write the // ZF flag. @@ -1464,9 +1480,7 @@ bool emitter::AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, GenCondition return id->idOpSize() == opSize; } - if ((cond.GetCode() == GenCondition::NE) || (cond.GetCode() == GenCondition::EQ) || - (cond.GetCode() == GenCondition::SLE) || (cond.GetCode() == GenCondition::SLT) || - (cond.GetCode() == GenCondition::SGE) || (cond.GetCode() == GenCondition::SGT)) + if ((cond.GetCode() == GenCondition::NE) || (cond.GetCode() == GenCondition::EQ)) { if (DoesWriteZeroFlag(lastIns) && IsFlagsAlwaysModified(id)) { @@ -1474,6 +1488,23 @@ bool emitter::AreFlagsSetToZeroCmp(regNumber reg, emitAttr opSize, GenCondition } } + if ((cond.GetCode() == GenCondition::SLT) || (cond.GetCode() == GenCondition::SGE)) + { + if (DoesWriteSignFlag(lastIns) && DoesWriteOverflowFlag(lastIns) && IsFlagsAlwaysModified(id)) + { + return id->idOpSize() == opSize; + } + } + + if ((cond.GetCode() == GenCondition::SGT) || (cond.GetCode() == GenCondition::SLE)) + { + if (DoesWriteZeroFlag(lastIns) && DoesWriteSignFlag(lastIns) && DoesWriteOverflowFlag(lastIns) && + IsFlagsAlwaysModified(id)) + { + return id->idOpSize() == opSize; + } + } + return false; } diff --git a/src/coreclr/jit/emitxarch.h b/src/coreclr/jit/emitxarch.h index 126d1a67a3593b..d9ee51d38171b5 100644 --- a/src/coreclr/jit/emitxarch.h +++ b/src/coreclr/jit/emitxarch.h @@ -767,6 +767,7 @@ bool IsDstSrcSrcAVXInstruction(instruction ins) const; bool IsThreeOperandAVXInstruction(instruction ins) const; static bool HasRegularWideForm(instruction ins); static bool HasRegularWideImmediateForm(instruction ins); +static bool DoesWriteOverflowFlag(instruction ins); static bool DoesWriteZeroFlag(instruction ins); static bool DoesWriteParityFlag(instruction ins); static bool DoesWriteSignFlag(instruction ins);