JIT: GetOperForHWIntrinsicId tests the isScalar pointer instead of *isScalar, dropping the constant in packed integer (cns - x)

[tannergooding]

While auditing gentree.cpp, I found that GenTreeHWIntrinsic::GetOperForHWIntrinsicId(bool* isScalar, bool getEffectiveOp) tests the isScalar pointer instead of the dereferenced value when refining a GT_SUB into a GT_NEG:

if (op1->IsVectorZero())
{
    oper = GT_NEG;
}
else if (isScalar && op1->IsCnsVec() && op1->AsVecCon()->IsScalarZero(simdBaseType))
{
    oper = GT_NEG;
}

isScalar is the out-param pointer and is always non-null, so this refinement (which is only valid for scalar ops, since IsScalarZero only checks the lowest element) also fires for packed ops. A packed subtract whose constant op1 has a zero low element but is not all-zero — e.g. Vector128.Create(0, 1, 2, 3) - x — is then reported as GT_NEG.

fgOptimizeHWIntrinsic consumes the effective oper for the (-v1) + v2 => v2 - v1 transform (morph.cpp), extracts Op(2) and discards Op(1) (the real constant). The result silently drops the constant for every lane whose value only mattered because the low lane happened to be zero.

Repro

using System.Runtime.CompilerServices;
using System.Runtime.Intrinsics;

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
static Vector128<int> Test(Vector128<int> v1, Vector128<int> v2)
    => (Vector128.Create(0, 1, 2, 3) - v1) + v2;

Vector128<int> r = Test(Vector128.Create(10), Vector128.Create(100));
// expected: <90, 91, 92, 93>
// actual:   <90, 90, 90, 90>   (constant lanes 1..3 dropped)

The fix is to dereference (*isScalar). This reproduces on current main and is latent (found by inspection, not a live report).

Note

This issue was authored with the help of GitHub Copilot.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.