On Wed, 23 Oct 2024 07:57:05 GMT, Jatin Bhateja <jbhat...@openjdk.org> wrote:
>> Jasmine Karthikeyan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address some changes from code review
>
> src/hotspot/share/opto/phaseX.cpp line 2301:
>
>> 2299: while(_igvn->_worklist.size() != 0) {
>> 2300: Node* n = _igvn->_worklist.pop();
>> 2301: Node* new_node = lower_node(n);
>
> _PhaseLowring::lower_node_ may do complex transformation where by replacing a
> graph pallet rooted at current node by another pallet. For each newly created
> node in new pallet, it should make sure to either directly run
> _igvn.transform, thereby triggering Ideal / Identity / Value sub-passed over
> it, OR insert the node into _igvn.worklist for lazy processing, in latter
> case you are consuming entire worklist after running over only Value
> transforms before existing the lowering phase.
I think we shouldn't run `Ideal` on the graph, because there is a chance that
it could undo the lowering changes that we just did. This gives lowering more
freedom to change the graph in different ways that would otherwise be undone by
Ideal. We run `Value` on all of the transformed nodes mainly so the types table
is accurate, so we can call upon the type of any node during lowering.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1814139289
