8355574: Fatal error in abort_verify_int_in_range due to Invalid CastII #25284
Conversation
|
👋 Welcome back qamai! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@merykitty The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
There was a problem hiding this comment.
CastLL related change looks good to me. ArrayCopyNode::get_partial_inline_vector_lane_count() looks reasonable but let's see what @jatin-bhateja thinks.
| } | ||
| return lane_count; | ||
| // As an optimization, choose the optimal vector size for bounded copy length | ||
| int ArrayCopyNode::get_partial_inline_vector_lane_count(BasicType type, jlong max_len) { |
There was a problem hiding this comment.
@jatin-bhateja you wrote this code. What do you think of the proposed change?
| const_len = lty->get_con() << shift; | ||
| int inline_limit = ArrayOperationPartialInlineSize / type2aelembytes(type); | ||
|
|
||
| const TypeLong* length_type = _igvn.type(length)->isa_long(); |
There was a problem hiding this comment.
Any particular benefit in eagerly pruning the block? It duplicates post-expansion GVN checks of the branch condition. (If it were normal parsing with prompt GVN analysis, you could detect the branch is dead right after generate_guard call.)
Alternatively, the checks are equivalent to checking that join of length_type with [0...inline_limit] is not empty. But I prefer to let GVN handle it.
There was a problem hiding this comment.
I think it is a trivial check and it is much more efficient than creating a bunch of nodes and removing them later.
|
Just a reminder that since this is a P4, the fix would need to be integrated until RDP 2 on Thursday (June 5) this week (or we need to raise the priority). |
|
@merykitty this pull request can not be integrated into git checkout misplacedcastll
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
openjdk
bot
added
the
merge-conflict
Hi,
The issue here is that the
CastLLNodeis created before the actual check that ensures the range of the input. This patch fixes it by moving the creation to the correct place, which is underinline_block. I also noticed that the code there seems incorrect and confusing.ArrayCopyNode::get_partial_inline_vector_lane_counttakes the length of the array, not the size in bytes. If you look into the method it will multiplyconst_lenwithtype2aelementbytes(bt)to get the size in bytes of the array. In the runtime test, we comparelength << log2(type2bytes(bt))withArrayOperationPartialInlineSize. This seems confusing, why don't we just comparelengthwithArrayOperationPartialInlineSize / type2bytes(bt), it also unifies the test with the actual cast.Please take a look and leave your reviews, thanks a lot.
Progress
Warnings
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25284/head:pull/25284$ git checkout pull/25284Update a local copy of the PR:
$ git checkout pull/25284$ git pull https://git.openjdk.org/jdk.git pull/25284/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25284View PR using the GUI difftool:
$ git pr show -t 25284Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25284.diff
Using Webrev
Link to Webrev Comment