We're going to need this for x64 as well, so we can shouldn't need to #ifdef it

I thought we have coverage in unit test for invalid immediate too. can you double check why this problem didn't appear in those tests?

We should validate that the GT_HWINTRINSIC node is marked GTF_SIDE_EFFECT when we insert the GT_BOUNDS_CHK node over op2 here (this should be happening already as part of GenTreeMultiOp::InitializeOperands which runs in the constructor).

Beyond that, we likely need to also ensure that hwintrinsic nodes marked as GTF_HW_USER_CALL are also marked as GTF_EXCEPT when the allowed input range is a partial range and for that to be stripped in rationalization (where we already strip GTF_HW_USER_CALL) if it is determined to be "in range".

DuplicateSelectedScalarToVector128 has HW_Flag_HasImmediateOperand and not any of the HW_Category_ flag in this method, but still it should check for the range.

/// <summary>
        /// uint8x16_t vdupq_lane_u8 (uint8x8_t vec, const int lane)
        ///   A32: VDUP.8 Qd, Dm[index]
        ///   A64: DUP Vd.16B, Vn.B[index]
        /// </summary>
        public static [Vector128](https://source.dot.net/System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector128_1.cs.html#a7d647b6968ad0f9)<byte> [DuplicateSelectedScalarToVector128](https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Arm/AdvSimd.PlatformNotSupported.cs,c98f4003aa811a2f)([Vector64](https://source.dot.net/System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector64_1.cs.html#576983e071d8f012)<byte> value, [[ConstantExpected](https://source.dot.net/System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis/ConstantExpectedAttribute.cs.html#12103fea5f68fa88)([Max](https://source.dot.net/System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis/ConstantExpectedAttribute.cs.html#bcd0dee62624224e) = (byte)(7))] byte index) { throw new [PlatformNotSupportedException](https://source.dot.net/System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/PlatformNotSupportedException.cs.html#ad07c3864de72561)(); }

It should, though I'm not sure the best way to cover all the intrinsics that have a possible ArgumentOutOfRangeException without going through them one by one (which would take some time). At least this gets most of it blocked and we can fix the others as they come in through Fuzzlyn.

can you open an issue for this?

can you elaborate on why we are doing this? Why was the repro working in Debug and not in Release?

Notably was talking about this with @jakobbotsch on Discord earlier and mentioned that we can't model ArgumentOutOfRangeException with OperExceptions because we can't reorder two such exceptions due to containing parameters.

I don't have the context as to why that is, but my guess is there's some other usage/handling of these that makes it problematic

What should we do instead?

I think this bit is overly conservative. We don't need to preserve any intrinsic with an immediate range, rather only ones that have something we can't determine is in bounds.

Consider for example if it was ShiftLeftLogicalSaturate(vr0, 1).

It is conservative, but I think that's ok for now. Given this fixes a correctness issue, we can figure out how to optimize it later.

Normally I'd agree, but this is going to mark a very large number of core APIs as side effecting even when they aren't and will likely have impact to important workloads.

Today, the general logic of "do I need a bounds check" goes through lookupImmBounds followed by addRangeCheckIfNeeded, so I'd expect us to be able to use this to mark the GT_HWINTRINSIC as side effecting and then key this liveness check based on that.

I'd expect if we do that, it also solves the issue of ensuring all nodes which insert bounds checks are marked that way, without needing to use this separate HasImmediateOperandRange check

For reference, here's where we check in rationalization for which operands are immediate and whether they are in range or not: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/rationalize.cpp#L457-L499

That then helps us decide whether or not we need to convert it back to a user call or if we can handle it "as is".

We have similar overall logic in the importer (https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/hwintrinsic.cpp and https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/hwintrinsicarm64.cpp, look for addRangeCheckIfNeeded), and I think we can mostly hook into the importation handling here to ensure this all works nicely and tracks the side effect.

OperMayThrow is expected to recompute whether we need to set GTF_EXCEPT on a node, without taking into account its operand nodes. This does take into account the operands nodes; I think instead the check should be:

(I don't think it makes any difference in practice since all GTF_HW_USER_CALL have GTF_EXCEPT, but still good to make the intent here clearer)