RFR: 8332827: [REDO] C2: crash in compiled code because of dependency on removed range check CastIIs

[Roland Westrelin]

On Wed, 11 Dec 2024 07:39:40 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> The failures that caused the backout were due to a bug in:
>> 
>> `find_or_make_integer_cast()`
>> 
>> which caused the `_range_check_dependency` field's value of the
>> existing cast node to not be set in the new cast node. I re-ran some
>> testing with this fixed and current jdk repo and found that a few
>> vectorization tests fail now because the patch pushes range check
>> `CastII` nodes through `AddI`/`SubI`. To fix this, I delayed that
>> transformation to after loop opts.
>
> src/hotspot/share/opto/castnode.cpp line 259:
> 
>> 257:   if (_range_check_dependency) {
>> 258:     if (phase->C->post_loop_opts_phase()) {
>> 259:       return this->in(1);
> 
> Does the removal of the CastII happen anywhere at all any more now?

Yes,during final graph reshape.

> test/hotspot/jtreg/compiler/rangechecks/TestRangeCheckCastIISplitThruPhi.java line 31:
> 
>> 29:  * @run main/othervm -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation TestRangeCheckCastIISplitThruPhi
>> 30:  *
>> 31:  *
> 
> Formatting is a little off. And would it make sense to add a run without flags?

I don't see anything wrong with the formatting. What am I missing?

> test/hotspot/jtreg/compiler/vectorization/TestVectorizationNegativeScale.java line 27:
> 
>> 25:  * @test
>> 26:  * @bug 8332827
>> 27:  * @summary [REDO] C2: crash in compiled code because of dependency on removed range check CastIIs
> 
> Can you say what happened with this test before your fix? Did it not vectorize? Or crash?

It's a test case for a failure I ran into while running tests. The root of the failure is some other bug that got fixed in the meantime. But the failure with this particular test case only happens with the change for this PR (and without the bug fix for the other bug). So while the test is not about this PR per se, I thought was interesting to keep the test anyway.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22568#discussion_r1880352614
PR Review Comment: https://git.openjdk.org/jdk/pull/22568#discussion_r1880347494
PR Review Comment: https://git.openjdk.org/jdk/pull/22568#discussion_r1880351082