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!
src/hotspot/cpu/arm/c2_lowering_arm.cpp line 29:
> 27: #include "opto/phaseX.hpp"
> 28:
> 29: Node* PhaseLowering::lower_node(Node* in) {
Suggestion:
Node* PhaseLowering::lower_node(Node* n) {
src/hotspot/cpu/ppc/c2_lowering_ppc.cpp line 29:
> 27: #include "opto/phaseX.hpp"
> 28:
> 29: Node* PhaseLowering::lower_node(Node* in) {
Suggestion:
Node* PhaseLowering::lower_node(Node* n) {
src/hotspot/cpu/riscv/c2_lowering_riscv.cpp line 29:
> 27: #include "opto/phaseX.hpp"
> 28:
> 29: Node* PhaseLowering::lower_node(Node* in) {
Suggestion:
Node* PhaseLowering::lower_node(Node* n) {
src/hotspot/cpu/s390/c2_lowering_s390.cpp line 29:
> 27: #include "opto/phaseX.hpp"
> 28:
> 29: Node* PhaseLowering::lower_node(Node* in) {
Suggestion:
Node* PhaseLowering::lower_node(Node* n) {
src/hotspot/cpu/x86/c2_lowering_x86.cpp line 29:
> 27: #include "opto/phaseX.hpp"
> 28:
> 29: Node* PhaseLowering::lower_node(Node* in) {
Suggestion:
Node* PhaseLowering::lower_node(Node* n) {
src/hotspot/share/opto/compile.cpp line 2466:
> 2464: print_method(PHASE_BEFORE_LOWERING, 3);
> 2465:
> 2466: PhaseLowering lower(&igvn);
Any specific reason to have lowering after loop optimizations ?
Lowered nodes may change the loop body size thereby impacting unrolling
decisions.
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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1812113082
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1812113953
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1812114516
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1812118894
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1812119441
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1812140992
PR Review Comment: https://git.openjdk.org/jdk/pull/21599#discussion_r1812110851
