ARM64: conditional compare rejects negative immediates the emitter can already encode as `ccmn`
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 2/5
- Tiempo estimado
- 1-3 horas
- Aptitud para principiantes
- 76/100
- Tipo de issue
- Error
- Claridad
- Bien especificado
- Estado de actividad
- Activo
- Stack tecnológico
- cpp
- Área
- compilers, performance
Línea de trabajo
Comience en src/coreclr/jit/emitarm64.cpp, en emitter::emitIns_valid_imm_for_ccmp, y luego inspeccione su llamada desde lower.cpp, así como los casos de inmediatos negativos en codegenarm64test.cpp. Confirme que los valores hasta -31 inclusive siguen la ruta existente del emisor ccmn, mientras que -32 sigue siendo rechazado, y ejecute las pruebas relevantes de generación de código ARM64 para verificar el límite y las formas de instrucción generadas.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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.
- Lenguaje dominante
- C#
- Estrellas
- 18.3k
- Forks
- 5.6k
- Merge medio
- 2 d 17 h
- PR fusionados (30 d)
- 615
Preparar el entorno
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de dotnet/runtime
-
area-Infrastructure-coreclr os-ios os-maccatalyst os-tvos untriaged
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
dotnet/runtime#134766 · 3 comentarios ·
Los mantenedores suelen responder en 1 día
-
area-System.Linq untriaged
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
dotnet/runtime#134736 · 3 comentarios ·
Los mantenedores suelen responder en 1 día
-
area-System.Numerics.Tensors untriaged
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
dotnet/runtime#134691 · 2 comentarios ·
Los mantenedores suelen responder en 1 día
-
area-System.Threading blocking-clean-ci Known Build Error untriaged
Dificultad 2/5 1-3 horas Aptitud para principiantes 70/100
dotnet/runtime#134679 · 4 comentarios ·
Los mantenedores suelen responder en 1 día
-
area-System.Security untriaged
Dificultad 2/5 1-3 horas Aptitud para principiantes 84/100
dotnet/runtime#134659 · 1 comentario ·
Los mantenedores suelen responder en 1 día
Todos los issues de dotnet/runtime
Issues similares
-
area-ai untriaged
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
dotnet/extensions#7790 ·
Los mantenedores suelen responder en 1 día
-
P2 testing
Dificultad 1/5 Menos de una hora Aptitud para principiantes 90/100
Los mantenedores suelen responder en 1 día
-
0 - Backlog Bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
BrighterCommand/Brighter#4444 ·
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
Los mantenedores suelen responder en 1 día
-
area:frontend bug FE P3
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
klasolsson81/jobbliggaren#1915 ·
Los mantenedores suelen responder en 1 día