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

[Quan Anh Mai]

On Thu, 6 Oct 2022 08:39:45 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   check index
>
> 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.

> 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.

> 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.

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

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