Repository navigation
Some hardware intrinsic nodes can reorder side effects #54956
Description
Reproduction:
using System.Numerics;
using System.Runtime.CompilerServices;
SideEffects(null, new Vector<int>[] { });
[MethodImpl(MethodImplOptions.NoInlining)]
static Vector<int> SideEffects(Vector<int>* a, Vector<int>[] b)
{
var v = Vector.AndNot(*a, b[0]);
return v;
}Actual result:
> dotnet run
Unhandled exception. System.IndexOutOfRangeException: Index was outside the bounds of the array.Expected result:
> dotnet run
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.This happens because of this code:
runtime/src/coreclr/jit/simdashwintrinsic.cpp
Lines 320 to 323 in e823eb6
Which does not account for side effects. Important note: since the importation code does not propagate the side effect flags properly (yet), they cannot be relied on when determining whether it is safe to swap at that point.
Possible solutions:
- Aggressively spill to temps - bad from CQ & TP standpoints.
- Use
GTF_REVERSE_OPS- bad from the standpoint that this flag should be removed. - Fix the operands up in LIR, where the tree walk no longer determines the execution order - probably cleanest but also most complex solution.
Note: the problematic code will become less centralized after #53450 is merged. See my review notes over there.
Activity
echesakov commentedon Mar 15, 2022
Un-assigning myself
cc @BruceForstall
AndyAyersMS commentedon Apr 28, 2022
Seems like we ought to fix this and should assign somebody.
Adding a regression test for dotnet#54956
Ensure side effects get spilled for the various hwintrinsic and simda…