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

[Cesar Soares Lucas]

On Thu, 31 Aug 2023 06:47:10 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Cesar Soares Lucas has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   move definitions to hpp files
>
> 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.

That's a good idea. Thanks!

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

Yeah, I actually tried that but the reuse of make_load/store wasn't justifiable without a much bigger refactor. I'll leave that refactor for another PR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15480#discussion_r1311821426
PR Review Comment: https://git.openjdk.org/jdk/pull/15480#discussion_r1311823254