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

Vladimir Kozlov kvn at openjdk.org
Tue Sep 19 22:39:41 UTC 2023


On Thu, 31 Aug 2023 21:56:07 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:
> 
>   Convert uses of AddPNode::make to new AddPNode

I don't like the last update. Now you may create AddP node even if offset is 0. Yes, AddPNode::Identity() will fix that but it will happen only later during IGVN transform. The only code which checks offset in BarrierSetC2::obj_allocate is hard to read.

The duplicated code was factored into `basic_plus_adr()` for that reason - to simplify code in uses.

May be we should put these changes on hold until after you look on JDK-8316560.

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

PR Comment: https://git.openjdk.org/jdk/pull/15480#issuecomment-1726631114


More information about the shenandoah-dev mailing list