JIT: propagate vector constants through assertion prop

[EgorBo]

Add a new kind of an assertion - O2K_CONST_VEC.

if (vec == Vector128<int>.Zero)
{
    // jit now constant propagates all `vec` uses with Vector128<int>.Zero here
}

Diffs

[Copilot]

Pull request overview

This PR extends CoreCLR JIT assertion propagation to recognize and propagate SIMD (vector) constants across equality/inequality guards, enabling more constant folding in common “guard then use” patterns for Vector64/128/256/512 integral vectors.

Changes:

• Added a new assertion operand kind O2K_CONST_VEC backed by an arena-allocated simd_t, including equality/printing/factory support.
• Taught optAssertionGenJtrue / assertion creation to recognize Vector*_op_Equality/op_Inequality patterns against GT_CNS_VEC and produce usable assertions (excluding FP base types).
• Implemented SIMD constant materialization during propagation (including VN-based propagation into SIMD-typed loads such as byref ABI patterns).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/coreclr/jit/compiler.h	Adds O2K_CONST_VEC plumbing to assertion descriptors, including storage, equality, and factory helpers; extends propagation eligibility for VN-based SIMD constants.
src/coreclr/jit/assertionprop.cpp	Recognizes SIMD compare patterns when generating assertions and propagates vector constants (including VN-based propagation for SIMD loads).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[github-actions]

🤖 Copilot Code Review — PR #127124

Note

This review was generated by Copilot and reflects analysis from multiple AI models (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: Well-motivated. Propagating vector constants through assertion prop enables further downstream optimizations (e.g., constant folding in HW intrinsic consumers). The pattern if (vec == Vector128<int>.Zero) { /* propagate Zero here */ } is real and common.

Approach: Follows existing assertion prop patterns for int/double constants, extending them cleanly to vector constants. Correctly handles IEEE floating-point semantics by restricting JTRUE-based assertions to integral SIMD base types.

Summary: ⚠️ Needs Human Review. The code logic appears correct across all paths (assertion creation, JTRUE normalization, propagation, type safety). There is a memory concern from inlining simd_t into the assertion descriptor union on x64, a stale comment, and no test coverage yet (understood for a draft PR). A human reviewer should evaluate whether the memory tradeoff (simplicity vs. per-assertion size increase on x64) is acceptable.

---

Detailed Findings

⚠️ Memory Impact — Inlining simd_t into AssertionDscOp2 union (advisory, flagged by all models)

compiler.h:8038: Changing from simd_t* (8 bytes on x64) to inline simd_t increases the AssertionDscOp2 union from 16 bytes (dominated by m_icon: ssize_t + FieldSeq*) to 64 bytes on x86/x64 (simd_t = simd64_t). This affects every assertion entry regardless of kind, not just vector assertions.

With 64–256 assertion table entries (optMaxAssertionCount), this adds roughly 3–12 KB per compilation on x64. On ARM64 (simd_t = simd16_t = 16 bytes), the increase is negligible since it matches the existing union max size.

The tradeoff: simpler code (no arena-allocated indirection) vs. memory overhead for the common non-vector case. The old simd_t* approach paid an arena-allocation cost only when creating vector assertions (rare), while the new approach pays the size cost on every assertion entry. May be an intentional simplification — would appreciate confirmation that the tradeoff is acceptable, especially for tiered compilation where many methods compile quickly.

⚠️ Stale Comment — compiler.h:8038 (flagged by all models)

simd_t        m_simdVal; // for O2K_CONST_VEC; arena-allocated (CMK_AssertionProp).

The comment still says "arena-allocated (CMK_AssertionProp)" but the value is now stored inline in the union. Should be updated to reflect inline storage.

✅ Correctness — JTRUE Equality/Inequality Normalization (assertionprop.cpp:1862–1917)

The equals logic correctly handles double-negation through all combinations:

• GT_EQ → equals = true, GT_NE → equals = false
• == 0 (equality intrinsic returning 0) flips equals
• op_Inequality flips equals again

All paths produce correct assertion kinds (OAK_EQUAL/OAK_NOT_EQUAL). The early swap at line 1856 (IsCnsIntOrI) and the inner swap at line 1903 (inside HW intrinsic block on hwi->Op(1)/Op(2)) operate on different scopes — after the HW intrinsic block, op1/op2 are completely reassigned from the intrinsic operands, so no interference.

✅ Correctness — Floating-Point SIMD Exclusion

The filter varTypeIsIntegral(hwi->GetSimdBaseType()) at line 1899 correctly prevents JTRUE-based assertions for floating-point SIMD types, where IEEE equality (+0 == -0, NaN != NaN) differs from bitwise equality.

The GT_CNS_VEC store path (lines 1146–1165) intentionally does not filter floats — store-based assertions record value identity (the local holds this exact bit pattern), not IEEE equality semantics. This is correct.

✅ Correctness — Type Safety in Propagation (assertionprop.cpp:3267–3281)

The propagation path correctly:

• Verifies the tree is SIMD-typed and matches the local's descriptor type
• Allocates a fresh GenTreeVecCon (since GenTreeVecCon and GenTreeLclVar have different node sizes)
• Uses genTypeSize(tree->TypeGet()) for memcpy — returns the correct SIMD width (8/12/16/32/64 bytes)
• Flows through optAssertionProp_Update which handles newTree != tree via parent ReplaceOperand
• VN is set correctly from op2 at line 3336

✅ Correctness — Struct Filter Updates (assertionprop.cpp:3605, 3664)

Changing varTypeIsStruct(tree) → varTypeIsStruct(tree) && !varTypeIsSIMD(tree) correctly allows SIMD types through global assertion prop while still blocking non-SIMD structs. This is consistent with the JIT type system where SIMD types satisfy both varTypeIsStruct and varTypeIsSIMD.

💡 Missing Profitability Heuristic (follow-up, not blocking)

The VN-based constant propagation path (optVNConstantPropOnTree) has a profitability heuristic (optIsProfitableToSubstitute) that avoids propagating vector constants when the parent HW intrinsic can't benefit. The assertion-prop path has no such heuristic — but this is consistent with how int/double are handled in the same path. Since vector constants are larger, the cost of unprofitable propagation is higher — may be worth a follow-up investigation.

💡 No Tests (expected for draft)

The PR is marked as draft and doesn't include tests. Before leaving draft status, tests should cover: assertion creation from stores, assertion creation from JTRUE branches, correct propagation, float exclusion, and type-mismatch bailout.

---

Models contributing: Claude Opus 4.6 (primary), Claude Sonnet 4.5, GPT-5.3-Codex.

Generated by Code Review for issue #127124 · ◷

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[tannergooding]

Is this likely to be needed?

We tend to push constants to the right fairly early and consistently across phases (import, VN, morph, etc), but then we still have a lot of checks in other phases, like this one, which presume they might still not have been fixed yet.

-- Not a big deal, mostly just a thought on how much potential throughput we might be wasting across the whole JIT with these repeated checks/handling.

[tannergooding]

The operand count check seems a bit superfluous and could probably be an assert. We should never encounter an equality node without 2 operands.

[tannergooding]

And same for the others

Suggested change

	if (op1->OperIs(GT_CNS_VEC))
	if (op1->IsCnsVec())

[tannergooding]

This also seems a bit superfluous and could be an assert. We should never be encountering IR where we a HWIntrinsic equality node where the operands aren't SIMD or are of different types.

It's the type of thing where something else has gone horribly wrong by that point.