RFR: 8373495: C2: Aggressively fold loads from objects that have not escaped [v17]
Quan Anh Mai
qamai at openjdk.org
Sat Jan 17 00:05:21 UTC 2026
On Fri, 16 Jan 2026 19:25:39 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:
>> @iwanowww I have run the latest version with tier1-tier4 and hs-comp-stress.
>> @dlunde Thanks for your comments, I have addressed them.
>
>> I have run the latest version with tier1-tier4 and hs-comp-stress.
>
> IMO tier1-tier4 is a bare minimum. For anything non-trivial I'd recommend to test it up to tier6. But in this particular case, it makes sense to test it up to tier8 (or even tier10) at least once to avoid any surprises after integration.
@iwanowww Thanks for your advice, I will perform more thorough testing when the reviews are somewhat content with the implementation.
>> I took a try, but I find it not compelling, the caching is the consequence of `find_previous_store` keeping walking from a node to its input. So, if a node does not observe that `base` has escaped, its input should not do so, either. Moving this logic to `LocalEA` seems not logical from that POV.
>
> `LocalEA` is already stateful and there are asserts in place to ensure that cached escape state agrees with current control (`local_ea.not_escaped_controls().member(ctrl)`). The assert can be turned into a dynamic check inside `MemNode::LocalEA::check_escape_status()` to report cached (non-escaping) state when queried control dominates cached one (and, hence, should be recorded in `LocalEA::_not_escaped_controls`). That should be equivalent to the current logic (modulo the dynamic check).
Do you mean adding an early return in `check_escape_control` when the queried control is a transitive input of the cached one like this:
if (_not_escaped_controls.member(ctl)) {
return NOT_ESCAPED;
}
I think it is correct to do so, but an assert that `_not_escaped_controls` does contain the `ctl` is a little bit stronger in terms of strictness. Moving this assert into `check_escape_status` will make it harder to reuse a `LocalEA` across multiple calls of `find_previous_store`. This is useful, for example, when the load is from a memory `Phi`, and we try to follow the `Phi` inputs to find the stored value along different paths of the merge.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/28812#issuecomment-3762253612
PR Review Comment: https://git.openjdk.org/jdk/pull/28812#discussion_r2700352410
More information about the hotspot-compiler-dev
mailing list