RFR: 8342662: C2: Add new phase for backend-specific lowering [v3]

Quan Anh Mai qamai at openjdk.org
Wed Oct 30 17:27:14 UTC 2024


On Wed, 30 Oct 2024 04:43:55 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:

>> Hi all,
>> This patch adds a new pass to consolidate lowering of complex backend-specific code patterns, such as `MacroLogicV` and the optimization proposed by #21244. Moving these optimizations to backend code can simplify shared code, while also making it easier to develop more in-depth optimizations. The linked bug has an example of a new optimization this could enable. The new phase does GVN to de-duplicate nodes and calls nodes' `Value()` method, but it does not call `Identity()` or `Ideal()` to avoid undoing any changes done during lowering. It also reuses the IGVN worklist to avoid needing to re-create the notification mechanism.
>> 
>> In this PR only the skeleton code for the pass is added, moving `MacroLogicV` to this system will be done separately in a future patch. Tier 1 tests pass on my linux x64 machine. Feedback on this patch would be greatly appreciated!
>
> Jasmine Karthikeyan 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 four additional commits since the last revision:
> 
>  - Merge branch 'master' into phase-lowering
>  - Remove platform-dependent node definitions, rework PhaseLowering implementation
>  - Address some changes from code review
>  - Implement PhaseLowering

src/hotspot/share/opto/phaseX.cpp line 2273:

> 2271: 
> 2272: Node* PhaseLowering::apply_ideal(Node* k, bool can_reshape) {
> 2273:   // Run the lowered Ideal method to continue doing transformations on the node, while avoiding existing transforms

Can you call `lower(k)` here and as a result, you can simply do `lower.optimize()` as the main entry?

src/hotspot/share/opto/phaseX.cpp line 2289:

> 2287:   _worklist.ensure_empty();
> 2288: 
> 2289:   C->identify_useful_nodes(_worklist);

To address @iwanowww 's concern, you can have a backend-specific method `PhaseLowering::do_lowering()` that will decide whether we should perform lowering on a particular graph.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1823087121
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1823088532


More information about the hotspot-compiler-dev mailing list