RFR: 8283699: Improve the peephole mechanism of hotspot [v4]

[Dean Long]

On Thu, 6 Oct 2022 13:10:08 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> src/hotspot/cpu/x86/peephole_x86.cpp line 28:
>> 
>>> 26: #ifdef COMPILER2
>>> 27: 
>>> 28: #include "opto/peephole.hpp"
>> 
>> I don't see why opto/peephole.hpp is useful.  Why not just include peephole_x86.hpp?  Then the empty peephole_<cpu>.hpp for the other platforms are no longer needed.
>
> `opto/peephole.hpp` is needed from the generated `ad_x86_peephole.cpp` so that `addI_rRegNode::peephole` can call the helper functions.

How about including `peephole_<cpu>.hpp` only when "peepprocedure" is seen, and delete opto/peephole.hpp and empty `peephole_<cpu>.hpp` files?

>> src/hotspot/cpu/x86/peephole_x86.cpp line 50:
>> 
>>> 48:       inst1 = inst0->in(1)->as_Mach();
>>> 49:       src1 = in;
>>> 50:     }
>> 
>> I don't understand why this optimization requires MachSpillCopy.  Is that the only time we sould see mov+add or mov+shift?
>
> Yes, `MachSpillCopy` is the node inserted by the register allocator to move values around. In this occasion, a move is inserted because the live range of the input overlaps with the live range of the output of an add or shift instruction.

OK.

>> src/hotspot/cpu/x86/peephole_x86.cpp line 132:
>> 
>>> 130:   cfg_->map_node_to_block(proj, nullptr);
>>> 131:   cfg_->map_node_to_block(root, block);
>>> 132: 
>> 
>> A lot of this seems like boiler-plate that could be refactored to make writing new peephole helpers simpler and less error-prone.
>
> While it seems to be a little boiler-plate, I think a general helper that removes, inserts an arbitrary number of nodes, and connects the graph correctly is hard to write. What do you think? Thanks a lot.

How does it work when there is no peepprocedure?  I would expect that all the boiler-plate details are taken care of by peepreplace.

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

PR: https://git.openjdk.org/jdk/pull/8025