ARM64: conditional compare rejects negative immediates the emitter can already encode as `ccmn`
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 76/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- cpp
- Domain
- compilers, performance
Research direction
Start in src/coreclr/jit/emitarm64.cpp at emitter::emitIns_valid_imm_for_ccmp, then inspect its call from lower.cpp and the negative-immediate cases in codegenarm64test.cpp. Confirm that values through -31 follow the existing ccmn emitter path while -32 remains rejected, and run the relevant ARM64 code-generation tests to verify the boundary and generated instruction forms.
Written by the indexing model from the issue text.
Description
Lowering::ContainCheckConditionalCompare decides whether the second compare of an ARM64 compare chain can hold its constant in the instruction by calling emitter::emitIns_valid_imm_for_ccmp. That predicate is a raw test of the 5-bit unsigned immediate field:
/*static*/ bool emitter::emitIns_valid_imm_for_ccmp(INT64 imm)
{
return ((imm & 0x01f) == imm);
}
so it accepts only 0...31 and rejects every negative constant. The emitter it gates has handled negative immediates for years: emitIns_R_I_FLAGS_COND does if (imm < 0) { ins = insReverse(ins); imm = -imm; } before validating isValidUimm<5>, insReverse maps INS_ccmp <-> INS_ccmn, and codegenarm64test.cpp already unit-tests INS_ccmp with −1, −2, −3, −5 and −31 under the comment "encoded as ccmn". The predicate was added as a field-width test and never widened to match the reversal in the emitter it guards.
As a result every compare chain whose second compare is against a small negative constant — in practice overwhelmingly x == -1 / x != -1 — materializes the constant with a movn into a scratch register and uses the register form of ccmp.
Minimal repro
using System;
using System.Runtime.CompilerServices;
public static class Program
{
[MethodImpl(MethodImplOptions.NoInlining)] public static bool NegImm(int x, int y) => (x > 10) & (y < -7);
[MethodImpl(MethodImplOptions.NoInlining)] public static bool NegImmLong(long x, long y) => (x > 10) & (y < -7);
[MethodImpl(MethodImplOptions.NoInlining)] public static bool NegEq(int x, int y) => (x > 10) & (y == -7);
[MethodImpl(MethodImplOptions.NoInlining)] public static bool NegBoundary31(int x, int y) => (x > 10) & (y < -31);
// Controls that must not change: -32 is not encodable, positives already worked.
[MethodImpl(MethodImplOptions.NoInlining)] public static bool NegBoundary32(int x, int y) => (x > 10) & (y < -32);
[MethodImpl(MethodImplOptions.NoInlining)] public static bool PosImm(int x, int y) => (x > 10) & (y < 7);
public static void Main() => Console.WriteLine(NegImm(11, -8));
}
Run with DOTNET_TieredCompilation=0 DOTNET_ReadyToRun=0 DOTNET_TieredPGO=0. The archived repro adds a PosImmLong control and an exhaustive self-check over x, y in [-40, 40]^2 (6,561 pairs per method).
Current codegen
osx-arm64, Release JIT at e4a207e6edf48b5a8611a0531352be025f86ba38, FullOpts:
; Assembly listing for method Program:NegImm(int,int):bool (FullOpts)
G_M000_IG02: ;; offset=0x0008
movn w2, #6
cmp w0, #10
ccmp w1, w2, z, gt
cset x0, lt
G_M000_IG03: ;; offset=0x0018
ldp fp, lr, [sp], #0x10
ret lr
; Total bytes of code 32 for method Program:NegImm(int,int):bool (FullOpts)
Expected codegen
; Assembly listing for method Program:NegImm(int,int):bool (FullOpts)
G_M000_IG02: ;; offset=0x0008
cmp w0, #10
ccmn w1, #7, z, gt
cset x0, lt
G_M000_IG03: ;; offset=0x0014
ldp fp, lr, [sp], #0x10
ret lr
; Total bytes of code 28 for method Program:NegImm(int,int):bool (FullOpts)
Impact
Target methods (measured, base vs. prototype): −16 bytes over four methods, each 32 -> 28 — NegImm, NegImmLong, NegEq, NegBoundary31. The three controls are byte-identical (NegBoundary32 stays 32; PosImm and PosImmLong stay 28). Both JITs print n=1530 from the exhaustive sweep with no mismatch, which is the arithmetically correct count for the four counted predicates over [-40, 40]^2, so the sweep really did exercise the changed paths.
Corpus (SuperPMI asmdiffs, osx/arm64, default collection set, 518,306 contexts / 150,768,836 base native bytes):
| Collection | Delta |
|---|---|
| aspire.nativeaot | −132 |
| aspnet2.run | −44 |
| benchmarks.run | −112 |
| benchmarks.run_pgo | −140 |
| benchmarks.run_pgo_optrepeat | −112 |
| libraries.crossgen2 | −288 |
| realworld.run | −72 |
| Total | −900 |
192 methods improved, 0 regressed — 159 at −4 bytes and 33 at −8, summing to −900. No context has a positive delta. Examples of the −8 methods: System.Reflection.TypeNameResolver:Resolve, System.Number:ComputeFloat[float], Microsoft.CodeAnalysis.CSharp.OverloadResolution:CheckForBadNonTrailingNamedArgument, System.Net.Http.HttpEnvironmentProxy:GetUriFromString.
Instruction-level audit across all seven diff files: removed 227 movn, 226 ccmp, 3 ldr, 3 csel, 3 mov, 2 and; added 225 ccmn, 1 ccmp, 4 mov, 3 csel, 2 ldr, 2 movn, 2 and. Net: 225 sites converted from movn + ccmp reg to ccmn #imm, matching 159 + 2x33 = 225 and the −900-byte total exactly. The few incidental mov/ldr/csel/and lines are register-allocation churn where one fewer register is live; none produced a size regression. All 192 diffs are FullOpts; no MinOpts context changed, consistent with containment being optimization-gated.
Be clear about the magnitude: −900 bytes is −0.0006% of the corpus baseline. The case for this change rests on it being a one-line predicate fix onto an already-implemented and already-unit-tested emitter path, not on aggregate size.
Measurement limitations.
- This is a code-size result only. Removing a
movnplausibly shortens the dependency chain, but no throughput or latency was measured and none is claimed. - Both JITs were Release builds, which do not compute PerfScore: totals are 0.0 and the relative geomean is exactly 1.0000 — no signal.
tpdiffwas not run (PIN is unavailable on osx/arm64).- The
-detailsCSVs were not archived for this candidate, so metrics beyond code size (JIT allocation, instruction counts) are unverified from the surviving artifacts. - Only osx/arm64 was measured; a Checked JIT was never built; no repo test suite was run.
- Note that the generated
codesize-summary.txtlabels its largest-delta listing "Top 25 regressions"; every entry in it is a −4 improvement. There are no regressions.
Notes
- Scope is ARM64 only. The predicate has a single call site in
lower.cpp, inside#if defined(TARGET_AMD64) || defined(TARGET_ARM64), which passes the signedtarget_ssize_tconstant through unchanged; the x64 CCMP (APX) predicate is a separateTARGET_AMD64-only definition inemitxarch.cppand is untouched. ARM32 is unaffected. - The
-32exclusion is load-bearing, not merely conservative.isValidUimm<5>is0 <= v < 32, so-31negates to an encodable31but-32negates to32, which is not encodable. In a Release JIT the emitter's "cannot be encoded" assert is compiled out, so admitting-32would silently emitccmn Rn, #0. TheNegBoundary32control verifies the boundary stays rejected. - Flag semantics:
CCMN Rn, #mcomputes the flags ofRn + m, and the sequence it replaces computesSUBS Rn, NOT(-m) + 1=Rn + m, so the adder sees the same integer sum and all of N, Z, C and V are bit-identical for signed and unsigned condition codes alike. This is an ISA-level argument, not a measurement: the repro covers signed<and==onintandlongonly. Unsigned (uint/ulong) compares and explicitly C/V-sensitive conditions (hs/lo/vs/vc) are not exercised, and SuperPMI compares generated assembly without executing it. Given that earliercmn-related correctness incidents (#113337/#113376, #118117/#118208) were in a different transform, this argument deserves reviewer scrutiny rather than assumption. - Code size is better here but not provably monotone: freeing a register causes regalloc churn, which could in principle regress a different corpus.
- Prior art, none duplicating this: #71616 introduced the predicate and #71705 / #79283 built the compare-chain lowering. An all-time search for
emitIns_valid_imm_for_ccmpreturns zero issues and only #71705.
Prototype patch
Experimental patch
diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp
index df354db94fe..b8075b490e8 100644
--- a/src/coreclr/jit/emitarm64.cpp
+++ b/src/coreclr/jit/emitarm64.cpp
@@ -2548,7 +2548,9 @@ emitter::code_t emitter::emitInsCode(instruction ins, insFormat fmt)
// true if this 'imm' can be encoded as a input operand to a ccmp instruction
/*static*/ bool emitter::emitIns_valid_imm_for_ccmp(INT64 imm)
{
- return ((imm & 0x01f) == imm);
+ // A negative immediate is encoded by reversing ccmp/ccmn and negating the value, so the
+ // representable range is symmetric around zero.
+ return (imm >= -31) && (imm <= 31);
}
// true if 'imm' can be encoded as an offset in a ldp/stp instruction
[!NOTE]
This issue was generated with GitHub Copilot.
- Dominant language
- C#
- Stars
- 18.3k
- Forks
- 5.6k
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 617
Getting set up
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from dotnet/runtime
-
area-System.Linq untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
dotnet/runtime#134736 · 3 comments ·
Maintainers usually reply within 1 day
-
area-System.Numerics.Tensors untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
dotnet/runtime#134691 · 2 comments ·
Maintainers usually reply within 1 day
-
area-System.Threading blocking-clean-ci Known Build Error untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
dotnet/runtime#134679 · 4 comments ·
Maintainers usually reply within 1 day
-
area-System.Security untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
dotnet/runtime#134659 · 1 comment ·
Maintainers usually reply within 1 day
-
area-System.Numerics untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
dotnet/runtime#134654 · 1 comment ·
Maintainers usually reply within 1 day
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
microsoft/fluentui-blazor#5364 ·
Maintainers usually reply within 1 day
-
.NET triage
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
microsoft/agent-framework#8811 ·
Maintainers usually reply within 1 day
-
.NET Docs
Difficulty 1/5 Under an hour Newbie friendliness 82/100
getsentry/sentry-dotnet#5637 · 1 comment ·
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
QuantConnect/Lean#9842 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
NethermindEth/nethermind#14012 ·
Maintainers usually reply within 1 day