RFR: 8256858: C2: Devirtualize PhaseIterGVN-specific methods

[Tobias Hartmann]

On Mon, 23 Nov 2020 13:48:21 GMT, Claes Redestad <redestad at openjdk.org> wrote:

> PhaseValues define the virtual method is_IterGVN, which is trivially returning 0(!) for all types except those derived from PhaseIterGVN. Similarly there's igvn_rehash_node_delayed which is virtual and a no-op in base types, but implemented to call rehash_node_delayed in PhaseIterGVN.
> 
> By devirtualizing we allow for more aggressive inlining and slightly better code generation in a few places. 
> 
> This increases sizeof(PhaseValues) from 2480 to 2488 on x64. Since we only go through a limited number of phases per compilation this seems acceptable.

Changes requested by thartmann (Reviewer).

src/hotspot/share/opto/addnode.cpp line 194:

> 192:       set_req(2, a22);
> 193:       progress = this;
> 194:       if (add2->outcnt() == 0 && phase->is_IterGVN()) {

Why did you remove the local variable `igvn`? Performance wise it shouldn't make a difference, right? And it's used below.

src/hotspot/share/opto/addnode.cpp line 629:

> 627:       set_req(Address, phase->transform(new AddPNode(in(Base),in(Address),add->in(1))));
> 628:       set_req(Offset, add->in(2));
> 629:       if (add->outcnt() == 0 && phase->is_IterGVN()) {

Again, I think replacing `igvn` by `phase->is_IterGVN()` makes the code less readable.

src/hotspot/share/opto/memnode.cpp line 2752:

> 2750: 
> 2751:   PhaseIterGVN* igvn = phase->is_IterGVN();
> 2752:   if (result != this && igvn != NULL) {

Here you did the reverse (replaced `phase->is_IterGVN()` by `igvn` local).

src/hotspot/share/opto/phaseX.hpp line 374:

> 372:   PhaseValues(PhaseValues* pt);
> 373:   NOT_PRODUCT(~PhaseValues();)
> 374:   PhaseIterGVN *is_IterGVN() { return (_iterGVN) ? (PhaseIterGVN*)this : NULL; }

`PhaseIterGVN *is_IterGVN` -> `PhaseIterGVN* is_IterGVN`

src/hotspot/share/opto/phaseX.hpp line 391:

> 389:   virtual ConNode* uncached_makecon(const Type* t);  // override from PhaseTransform
> 390: 
> 391:   const Type* saturate(const Type* new_type, const Type* old_type,

Indentation should be fixed (in the next line).

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

PR: https://git.openjdk.java.net/jdk/pull/1385