RFR: JDK-8315279: Factor 'basic_plus_adr' out of PhaseMacroExpand and delete make_load/store [v2]

[Christian Hagedorn]

On Thu, 31 Aug 2023 00:58:44 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:

>> I believe the factory methods for AddPNode should be in the AddPNode class. The make_load / make_store methods in PhaseMacroExpand can be refactored to instead just use the "make" methods from Load/Store classes.
>> 
>> Tested with tier1-3.
>
> Cesar Soares Lucas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   move definitions to hpp files

Otherwise, the clean-up looks good.

src/hotspot/share/opto/addnode.hpp line 177:

> 175: 
> 176:   // Helper methods roughly modeled after GraphKit:
> 177:   static Node* make(PhaseIterGVN* igvn, Node* base, Node* ptr, Node* offset) {

I suggest to pass `igvn` as reference here (i.e. `PhaseIterGVN&`) and below. Then you can avoid to use `&igvn()` in the code and just pass it as reference directly.

src/hotspot/share/opto/macro.cpp line 1683:

> 1681: 
> 1682:   Node* memadr = AddPNode::make(&_igvn, object, oopDesc::mark_offset_in_bytes());
> 1683:   rawmem = transform_later(StoreNode::make(_igvn, control, rawmem, memadr, nullptr, mark_node, TypeX_X->basic_type(), MemNode::unordered));

You could still think about keeping `make_load/store()` and just replace `basic_plus_adr()` with `AddPNode::make()` there which might make it easier to read the code here. But both is fine and I leave it up to you to decide.

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15480#pullrequestreview-1604052768
PR Review Comment: https://git.openjdk.org/jdk/pull/15480#discussion_r1311168043
PR Review Comment: https://git.openjdk.org/jdk/pull/15480#discussion_r1311173320