[Jit] Indexing mod array.Length not recognized as bounds safe

[benaadams]

e.g.

var i = buckets[hashCode % buckets.Length];

Will introduce a range check, even though its always safe (though maybe not for Length == 0?)

category:cq
theme:bounds-checks
skill-level:intermediate
cost:small
impact:small

[mikedn]

though maybe not for Length == 0

x % 0 throws exception so the array access cannot happen anyway.

[benaadams]

Perfect then :)

[mikedn]

I have a prototype at mikedn/coreclr@a587bd3

The main problem is that it relies on GT_MOD being present when assertion propagation runs. But on ARM GT_MOD is morphed into x - (x / y) * y (because ARM doesn't have such an instruction) and that is difficult to recognize as x % y.

Now, it should be possible to delay the transformation of GT_MOD until lowering. But doing so prevents the potential CSE of x / y in cases where there's an x / y already available.

So I don't know, maybe having this work only on x86/x64 is better than nothing.

[AndyAyersMS]

I wonder if this is an opportunity to use the wrapper concept like in #9056, though it perhaps would be too clunky when the underlying tree shape varies across architectures.

But I think firing for x64/x86 only is ok here.

[mikedn]

I wonder if this is an opportunity to use the wrapper concept like in #9056, though it perhaps would be too clunky when the underlying tree shape varies across architectures.

I don't know, seems a bit too convoluted for what is otherwise a simple expression.

But I think firing for x64/x86 only is ok here.

Yes, considering that it doesn't save much probably it's ok. Though I'm a bit concerned about future interactions with #5213 though, it seems that one possible way to implement that would be to take advantage or morph+CSE even on x64/x86. Otherwise it would problematic for lowering to find div/mod nodes that can generate a single div instruction.

[benaadams]

It should drop 2 range checks from this PR to 1 range check dotnet/coreclr#15462 (3 in PR) from 6 in dotnet/master