[clr-interp] Matrix4x4 tests fail on DOTNET_InterpMode=1

[davidwrighton]

On DOTNET_InterpMode=1 these tests fail because the underlying SIMD api uses Single.MultiplyAddEstimate which behaves differently than the expected path when jitted vs R2R'd

[davidwrighton]

I don't think we should fix this.

[dotnet-policy-service]

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

[tannergooding]

The R2R path should be marking these appropriately, so they are forced to compile at runtime. We generally document this under https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/vectors-and-intrinsics.md#non-deterministic-intrinsics-in-systemprivatecorelib

See also https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/importercalls.cpp#L9850-L9867, which should be hitting:

    if (BlockNonDeterministicIntrinsics(mustExpand))
    {
        return nullptr;
    }

This in turn is implemented as:

    bool BlockNonDeterministicIntrinsics(bool mustExpand)
    {
        // We explicitly block these APIs from being expanded in R2R
        // since we know they are non-deterministic across hardware

        if (IsReadyToRun())
        {
            if (mustExpand)
            {
                implLimitation();
            }
            return true;
        }
        return false;
    }

That is, they should explicitly be left in as calls and not generated in the R2R image. Instead, they should end up always being jitted and so should always return the same result at runtime.

[BrzVlad]

@tannergooding In our case, on ios, it would mean that these would be interpreted (so their execution would be orders of magnitude slower). Would you say that this is acceptable ? Do you think these would be performance critical ?

[tannergooding]

In our case, on ios, it would mean that these would be interpreted (so their execution would be orders of magnitude slower). Would you say that this is acceptable ?

I think its a matter of correctness and so a bit irrelevant. We cannot have functions, particularly "core math" functions differing in result between different modes (AOT vs R2R vs JIT vs Interpreter) within the same process.

Do you think these would be performance critical ?

The estimate functions primarily exist for "performance" related reasons. That is, they intentionally exist to say give me the faster of FusedMultiplyAdd(x, y, z) or (x * y) + z. I wouldn't think it should be terribly difficult for the interpreter to handle these itself. That being said, I also wouldn't expect them to be all that typical in code that ends up being interpreted in practice.

[BrzVlad]

Thanks for the explanation. Seems like this issue reproduces on our tests due to misconfiguration. On iOS we should be fine since we would only use the interpreter path and it doesn't seem to be perf critical as of right now. Triaging as future milestone.