8384099: C2: assert(_base == Long) failed on 32-bit systems#31076
8384099: C2: assert(_base == Long) failed on 32-bit systems#31076bulasevich wants to merge 1 commit intoopenjdk:masterfrom
Conversation
|
👋 Welcome back bulasevich! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@bulasevich 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. |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
shipilev
left a comment
There was a problem hiding this comment.
I looked around to see how safe it is, and have not been able to prove it.
We do not lose data here with P2X conversion (32bit -> 32bit), and I2L promotion should not hurt here. I do wonder if a X2P (really L2P?) conversion somewhere earlier actually loses the data when casting to 32-bit P...
| @@ -1345,6 +1345,12 @@ Node* VPointer::make_pointer_expression(Node* iv_value, Node* ctrl) const { | |||
| variable = new CastP2XNode(ctrl, variable); | |||
| phase->register_new_node(variable, ctrl); | |||
| } | |||
| NOT_LP64( // On 32-bit CastP2X produces TypeInt | |||
There was a problem hiding this comment.
NOT_LP64 is really for single-line use. For multi-line, wrap it in #ifndef _LP64.
|
I suspect we are encountering "int that pretends to be in long group", because we are missing a case somewhere here: jdk/src/hotspot/share/opto/mempointer.cpp Lines 255 to 291 in d81632a |
On 32-bit systems, CastP2X returns TypeInt instead of TypeLong, triggering assert(_base == Long) in MulLNode. Added ConvI2LNode (guarded by NOT_LP64) to fix the type mismatch:
MulLNode(CastP2XNode)->MulLNode(ConvI2LNode(CastP2XNode))Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/31076/head:pull/31076$ git checkout pull/31076Update a local copy of the PR:
$ git checkout pull/31076$ git pull https://git.openjdk.org/jdk.git pull/31076/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 31076View PR using the GUI difftool:
$ git pr show -t 31076Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/31076.diff
Using Webrev
Link to Webrev Comment