RFR: 8372779: C2: Disambiguate Node::adr_type for the IR graph [v3]
Quan Anh Mai
qamai at openjdk.org
Wed Dec 10 17:45:10 UTC 2025
On Sun, 7 Dec 2025 12:12:20 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:
>> Hi,
>>
>> Currently, `Node::adr_type` is 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 introducing `Node::in_adr_type` and `Node::out_adr_type` that refer to those properties, respectively. It also introduces a local verification of the memory graph during compilation. These additions uncover some issues:
>>
>> - Sometimes, the memory is wired incorrectly, such as in `LibraryCall::extend_setCurrentThread`, the `Phi` collect the `StoreNode`s instead of the whole memory state. I think these issues do not result in crashes or miscompilation, though.
>> - `AryEqNode` reports `adr_type` being `TypeAryPtr::BYTES` (it inherits this from `StrIntrinsicNode`). This is incorrect, however, as it can accept `char[]` inputs, too.
>> - For nodes such as `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_allocation` uses a raw `ProjNode` as the base for a `MergeMem`, 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.
>
> Quan Anh Mai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>
> - Merge branch 'master' into adrtype
> - store_to_memory does not emit MemBars
> - Disambiguate Node::adr_type
For the other issues (`AryEqNode` advertises incorrect `adr_type` and `LibraryCall::extend_setCurrentThread` incorrectly wires the memory nodes), there is no immediate issue apart from incorrectly looking graph, so I have not come up with a real failure.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/28570#issuecomment-3638245320
More information about the shenandoah-dev
mailing list