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

Vladimir Ivanov vlivanov at openjdk.org
Thu Aug 31 20:59:09 UTC 2023


On Thu, 31 Aug 2023 20:32:56 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:

>> static Node* make(PhaseIterGVN& igvn, Node* base, Node* ptr, Node* offset) {
>>     return igvn.register_new_node_with_optimizer(new AddPNode(base, ptr, offset));
>>   }
>> 
>> vs
>> 
>>   static Node* make(PhaseIterGVN& igvn, Node* base, int offset) {
>>     return (offset == 0) ? base : make(igvn, base, base, (Node*) igvn.makecon(offset));
>>   }
>> 
>> 
>> IMO latter case adds more confusion than improves readability. Why not  unconditionally call `AddPNode::make(base, base, offset)` instead?
>
> Thanks for clarifying @iwanowww . I think I see your point now. My original intent was to just move these methods out of PhaseMacroExpand and not much else. 
> 
> I'm going to do some more refactoring and patch all users of these make methods to just use this single method: `static Node* make(PhaseIterGVN& igvn, Node* base, Node* ptr, Node* offset)`. What do you think?

Sounds good. 

But in the future I'd like to see `PhaseMacroExpand` and `PhaseIdealLoop` migrated to `GraphKit` instead.

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

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


More information about the shenandoah-dev mailing list