RFR: 8373591: C2: Fix the memory around some intrinsics nodes [v5]

[Quan Anh Mai]

On Fri, 30 Jan 2026 02:38:57 GMT, Dean Long <dlong at openjdk.org> wrote:

>> 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 seven additional commits since the last revision:
>> 
>>  - Merge branch 'master' into intrinsicsadrtype
>>  - copyright year
>>  - Merge branch 'master' into intrinsicsadrtype
>>  - consolidate the memory effect into a function
>>  - Use MemBar instead of widening the intrinsic memory
>>  - Fix Shenandoah
>>  - Fix memory around intrinsics nodes
>
> This may be unrelated, but I checked to see if we treat Op_EncodeISOArray the same as Op_StrCompressedCopy everywhere.  In two places in `ConnectionGraph::split_unique_types`, we treat them differently.  For both we look at in(MemNode::Memory), but for Op_EncodeISOArray we also look at use->in(3).  I don't understand this code well enough to decide if this a missing optimization or a correctness issue.

@dean-long Thanks for taking a look.

> So I tried adding a test for it in TestAntiDependency.java. But to my surprise, it passes, even without the fixes in this PR

I have added a test for this method. If it does not fail then adding `-XX:+StressGCM -XX:+StressLCM` may help.

> Dumb question: why are these intrinsic nodes not implemented as MemNodes?

I think it is because only `LoadNode` and `StoreNode` are `MemNode`, even `LoadStoreNode` does not extend `MemNode`.

> > For nodes such as StrInflatedCopyNode, as they consume more than they produce, during scheduling, we need to compute anti-dependencies. This is not the case
>
> Why is that?

During `PhaseIdealLoop::get_late_ctrl`, we only check the anti-dependency when a node returns `true` for `is_Load()`:

      if (n->is_Load() && LCA != early) {
        LCA = get_late_ctrl_with_anti_dep(n->as_Load(), early, LCA);
      }

During `PhaseCFG::schedule_late`, we only check the anti-dependency when a node has the flag `Flag_needs_anti_dependence_check` set.

    bool Node::needs_anti_dependence_check() const {
      if (req() < 2 || (_flags & Flag_needs_anti_dependence_check) == 0) {
        return false;
      }
      return in(1)->bottom_type()->has_memory();
    }

We may fix these places, but since it is a really rare occurrence that a node consumes some memory and produces some but the latter is different from the former, so it is more reasonable to fix the graph at these nodes.

>> so we should fix it by making the nodes kill all the memory they consume.
>
> Why can't we use MergeMem and memory slices/aliases like regular load and store?

Thanks to Roland's suggestion, now it only kills the 2 slices it concerns with and not the whole memory state.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/28789#issuecomment-3821512600