Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics
See info in area-owners.md if you want to be subscribed.

@copilot please evaluate and address the feedback

🤖 Copilot Code Review — PR #126424

Holistic Assessment

Motivation: Vectorizing Acosh for SIMD vector types and TensorPrimitives is well-motivated — it follows the established pattern from Asin (already vectorized) and fills a TODO: Vectorize gap in the codebase. The mathematical approach is sound and the implementation is numerically careful.

Approach: The implementation correctly uses the identity acosh(x) = log(x + sqrt(x² − 1)) with special-case handling for values near 1 (to avoid catastrophic cancellation) and large values (to avoid overflow). The float path widens to double for accuracy, consistent with other functions. The overall pattern closely follows the existing Asin vectorization.

Summary: ⚠️ Needs Human Review. The code is mathematically correct and follows established patterns. However, the new public API surface in the ref file is not in alphabetical order, and I could not locate a linked api-approved issue (which may exist but is not referenced in the PR). A human reviewer should confirm these points.

Detailed Findings

⚠️ Ref File Ordering — Acosh placed after Asin instead of before

In src/libraries/System.Runtime.Intrinsics/ref/System.Runtime.Intrinsics.cs, the new Acosh entries are placed after Asin in all four Vector types (Vector64, Vector128, Vector256, Vector512). Alphabetically, Acosh (Ac...) should come before Asin (As...), since 'c' < 's'.

Current order:

AnyWhereAllBitsSet
Asin(double)
Asin(float)
Acosh(double)  ← new
Acosh(float)   ← new
AsByte
```

Expected alphabetical order:
```
AnyWhereAllBitsSet
Acosh(double)
Acosh(float)
Asin(double)
Asin(float)
AsByte

The same ordering issue exists in the implementation files (Vector64.cs, Vector128.cs, Vector256.cs, Vector512.cs), where Acosh methods are placed after Asin methods but should precede them alphabetically — consistent with the existing pattern where Asin comes before Cos which comes before Sin.

⚠️ API Approval — No linked api-approved issue found

This PR adds 8 new public API methods (Acosh for float and double across Vector64/128/256/512) with corresponding ref assembly entries. Per dotnet/runtime process, new public APIs typically require a linked issue with the api-approved label. The commit message and PR branch contain no issue references (Fixes #N, Closes #N). This may be covered by a broader vectorization tracking issue or batch approval — a human reviewer should confirm.

✅ Mathematical Correctness — All identities verified

The AcoshDouble implementation is mathematically sound:

• Normal case log(x + sqrt(x² − 1)): Standard identity, verified correct.
• Near-one case sqrt(2(x − 1)) for x ≤ 1 + 2⁻²⁵: Valid first-order Taylor approximation of acosh(1 + ε) ≈ sqrt(2ε). The threshold 2⁻²⁵ is well-chosen to prevent catastrophic cancellation in x² − 1 while the approximation error remains < 2.5e-9 relative.
• Large case ln(2) + ln(x) for x > 2²⁸: Correct asymptotic identity. The threshold 2²⁸ is more conservative than AMD AOCL's 1/sqrt(ε) (~6.7e7), which is fine.
• Domain check x < 1 → NaN: Correct. x = 1 correctly falls through to nearOneMask producing sqrt(0) = 0 = acosh(1).
• ConditionalSelect ordering: nanMask applied last (highest priority) correctly overrides all other results for out-of-domain inputs. No mask overlap between nearOneMask and largeMask ranges.
• SIMD-correct: All paths compute eagerly (as required by branchless SIMD), and invalid intermediate results (e.g., sqrt(negative) → NaN for x < 1) are properly masked out by ConditionalSelect.

✅ Pattern Consistency — Matches existing Asin vectorization

The Acosh methods in Vector64/128/256/512 follow the exact same structure as the existing Asin methods:

• Hardware-accelerated path calls into VectorMath.AcoshDouble/AcoshSingle
• Fallback path decomposes into lower/upper halves
• Float path checks for next-wider hardware acceleration (Vector128.IsHardwareAccelerated for Vector64 float, etc.)
• Vector64<T>.Acosh<T> scalar fallback loop uses same pattern as Asin<T>

✅ Test Coverage — Appropriate tolerances

The test tolerance change from null (exact match) to DetermineTolerance(doubleTolerance: 1e-14, floatTolerance: 1e-6f) is appropriate and consistent with similar vectorized functions (Cosh uses 1e-14 double, Exp2 uses 1e-14/1e-5f).

💡 AcoshSingle Attribution Comment — Slightly misleading

The AcoshSingle comment states "This code is based on acoshf from amd/aocl-libm-ose" and describes the AMD three-range approach (including log1p), but the actual implementation simply widens to double and calls AcoshDouble. The code is inspired by the AMD approach (which also widens to double for float accuracy) but doesn't implement the described algorithm. The comment also lists a copyright range of "2008-2022" while THIRD-PARTY-NOTICES.TXT says "2008-2020". Consider clarifying that the implementation uses widening-to-double rather than the described multi-range approach directly.

💡 AcoshOperator Visibility — private → internal

AcoshOperator<T> was changed from private to internal. The pattern is mixed across operators — AsinhOperator, AtanOperator, AtanhOperator are internal, while AsinOperator, AcosOperator are private. Not a blocking concern, but worth noting the inconsistency with the closely related AsinOperator (which is vectorized and private).

Generated by Code Review for issue #126424 · ◷

@copilot please evaluate and address the feedback

Addressed all three feedback items in 3544d66:

1. Alphabetical ordering: Moved Acosh before Asin at all 4 locations in the ref file.
2. Near-one accuracy: Replaced x²-1 with (x-1)*(x+1) to avoid catastrophic cancellation near x=1, and removed the inaccurate sqrt(2*(x-1)) fast-path entirely — the reformulated expression handles values near 1 accurately since LogDouble internally uses a log1p polynomial for reduced arguments near 1.
3. Misleading comment: Updated the AcoshSingle comment to accurately describe the widen-to-double implementation.

All 1575 TensorPrimitives tests pass (including Acosh with 1e-14 double tolerance).