Hacktoberfest 2026: những issue maintainer đã đánh dấu cho tháng Mười, đang mở và phù hợp người mới. Xem issue Hacktoberfest

ARM64: conditional compare rejects negative immediates the emitter can already encode as `ccmn`

Đang mở Phù hợp với người mới
#134,663 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

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ả

area-CodeGen-coreclr performance

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 movn plausibly 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.
  • tpdiff was not run (PIN is unavailable on osx/arm64).
  • The -details CSVs 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.txt labels 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 signed target_ssize_t constant through unchanged; the x64 CCMP (APX) predicate is a separate TARGET_AMD64-only definition in emitxarch.cpp and is untouched. ARM32 is unaffected.
  • The -32 exclusion is load-bearing, not merely conservative. isValidUimm<5> is 0 <= v < 32, so -31 negates to an encodable 31 but -32 negates to 32, which is not encodable. In a Release JIT the emitter's "cannot be encoded" assert is compiled out, so admitting -32 would silently emit ccmn Rn, #0. The NegBoundary32 control verifies the boundary stays rejected.
  • Flag semantics: CCMN Rn, #m computes the flags of Rn + m, and the sequence it replaces computes SUBS 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 == on int and long only. 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 earlier cmn-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_ccmp returns 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

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. 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.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của dotnet/runtime

Tất cả issue của dotnet/runtime

Issue tương tự

Thêm issue về C#

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.