On Mon, 21 Oct 2024 04:11:03 GMT, Jasmine Karthikeyan
<jkarthike...@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!
Build changes look good (but would be slightly better without the extra blank
line). I have not reviewed the actual hotspot changes.
make/hotspot/gensrc/GensrcAdlc.gmk line 60:
> 58:
> 59: ADLC_CFLAGS += -D$(HOTSPOT_TARGET_CPU_DEFINE)
> 60:
Maybe skip this blank line?
src/hotspot/cpu/aarch64/c2_lowering_aarch64.cpp line 31:
> 29: Node* PhaseLowering::lower_node(Node* in) {
> 30: return nullptr;
> 31: }
Note that this, and several other of the new files, are missing a trailing
newline on the last line (marked by the red circle/dash icon). I thought this
was checked by jcheck, but apparently not. It is still not recommended, though.
-------------
Marked as reviewed by ihse (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/21599#pullrequestreview-2381995734
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1808752950
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1808763526
