8372779: C2: Disambiguate Node::adr_type for the IR graph #28570
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
Currently,
Node::adr_typeis ambiguous. For some, it refers to the memory the node consumes, while for the others, it refer to the memory the node produces. This PR removes that ambiguity by introducingNode::in_adr_typeandNode::out_adr_typethat refer to those properties, respectively. It also introduces a local verification of the memory graph during compilation. These additions uncover some issues:LibraryCall::extend_setCurrentThread, thePhicollect theStoreNodes instead of the whole memory state. I think these issues do not result in crashes or miscompilation, though.AryEqNodereportsadr_typebeingTypeAryPtr::BYTES(it inherits this fromStrIntrinsicNode). This is incorrect, however, as it can acceptchar[]inputs, too.StrInflatedCopyNode, as it consumes more than it produces, during scheduling, we need to compute anti-dependencies. This is not the case, so I fixed it by making it kill all the memory it consumes.GraphKit::set_output_for_allocationuses a rawProjNodeas the base for aMergeMem, this is really suspicious. I didn't fix it, as it seems to not result in any symptom at the moment.In the end, the execution of the compiler is strictly more restricted than before, and there is less room for ambiguity.
Please take a look and leave your reviews, thanks a lot.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28570/head:pull/28570$ git checkout pull/28570Update a local copy of the PR:
$ git checkout pull/28570$ git pull https://git.openjdk.org/jdk.git pull/28570/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28570View PR using the GUI difftool:
$ git pr show -t 28570Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28570.diff
Using Webrev
Link to Webrev Comment