ARM64: conditional compare rejects negative immediates the emitter can already encode as `ccmn`
Maintainer thường phản hồi trong vòng 1 ngày
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 2/5
- Thời gian dự kiến
- 1-3 giờ
- Mức phù hợp với người mới
- 76/100
- Loại issue
- Lỗi
- Độ rõ ràng
- Đặc tả rõ ràng
- Mức độ hoạt động
- Sôi nổi
- Công nghệ
- cpp
- Lĩnh vực
- compilers, performance
Hướng nghiên cứu
Bắt đầu tại src/coreclr/jit/emitarm64.cpp, ở emitter::emitIns_valid_imm_for_ccmp, sau đó kiểm tra lệnh gọi của nó từ lower.cpp cũng như các trường hợp immediate âm trong codegenarm64test.cpp. Xác nhận rằng các giá trị đến -31, bao gồm cả -31, tuân theo đường đi emitter ccmn hiện có, trong khi -32 vẫn bị từ chối, và chạy các bài kiểm thử tạo mã ARM64 liên quan để xác minh ranh giới và các dạng lệnh được tạo ra.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
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.
- Ngôn ngữ chính
- C#
- Star
- 18.3k
- Fork
- 5.6k
- Merge trung bình
- 2 ngày 17 giờ
- Pull request đã merge (30 ngày)
- 615
Chuẩn bị môi trường
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của dotnet/runtime
-
area-System.Numerics.Tensors untriaged
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
dotnet/runtime#134691 · 2 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
area-System.Threading blocking-clean-ci Known Build Error untriaged
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
dotnet/runtime#134679 · 4 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
area-System.Security untriaged
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 84/100
dotnet/runtime#134659 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
area-System.Numerics untriaged
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 88/100
dotnet/runtime#134654 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
area-Interop-coreclr untriaged
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
dotnet/runtime#134617 · 2 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
Tất cả issue của dotnet/runtime
Issue tương tự
-
0 - Backlog Bug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 88/100
BrighterCommand/Brighter#4444 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
bug
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 88/100
Maintainer thường phản hồi trong vòng 1 ngày
-
area:frontend bug FE P3
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 86/100
klasolsson81/jobbliggaren#1915 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
-
triage
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 76/100
microsoft/vscode-copilotstudio#431 ·
Maintainer thường phản hồi trong vòng 2 ngày