Skip to content

Rework profiled to nop guard upgrades in VP - #6752

Merged
vijaysun-omr merged 1 commit into
eclipse-omr:masterfrom
jdmpapin:vp-guard-upgrade
Oct 6, 2022
Merged

Rework profiled to nop guard upgrades in VP#6752
vijaysun-omr merged 1 commit into
eclipse-omr:masterfrom
jdmpapin:vp-guard-upgrade

Conversation

@jdmpapin

@jdmpapin jdmpapin commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

For all profiled guards (VFT test or method test), when the guard is not folded, replace it with a noppable guard if possible.

Replacing a VFT test with a noppable guard is possible when there is a type bound C for the receiver, and C is the type expected by the VFT test, and C has not been extended. It's as though C is final (which would allow folding via a fixed-type constraint), but only until further notice.

Replacing a method test with a noppable guard is possible when there is a type bound C for the receiver, and C has the expected method, and the expected method is not overridden. It's as though the expected method is final (which would allow folding), but only until further notice.

By default such upgrades are performed only outside of the VM startup phase, since during startup the VM is still loading classes pretty frequently, so the non-extended or nonoverridden state is not as significant. To enable the transformation during startup, set the environment variable TR_upgradeToNopGuardDuringStartup.

A new query VMEnv::isVMInStartupPhase(TR::Compilation*) is defined to detect this startup phase. It is intended for downstream projects to override. The default implementation arbitrarily returns false.

There was pre-existing code that attempted to change profiled guards into nop guards, but it worked only for VFT tests, and only when the cold call was a virtual call (although part of the code appeared to be trying to handle interface calls). The new implementation also works for method tests and for interface calls, and in the case of method test, the logic that triggers the transformation is largely shared with other recent improvements to the handling of method test. As such, this commit deletes the prior implementation of this concept.

@vijaysun-omr

Copy link
Copy Markdown
Contributor

This change looks good to me.

@vijaysun-omr

Copy link
Copy Markdown
Contributor

Jenkins build all

For all profiled guards (VFT test or method test), when the guard is not
folded, replace it with a noppable guard if possible.

Replacing a VFT test with a noppable guard is possible when there is a
type bound C for the receiver, and C is the type expected by the VFT
test, and C has not been extended. It's as though C is final (which
would allow folding via a fixed-type constraint), but only until further
notice.

Replacing a method test with a noppable guard is possible when there is
a type bound C for the receiver, and C has the expected method, and the
expected method is not overridden. It's as though the expected method is
final (which would allow folding), but only until further notice.

By default such upgrades are performed only outside of the VM startup
phase, since during startup the VM is still loading classes pretty
frequently, so the non-extended or nonoverridden state is not as
significant. To enable the transformation during startup, set the
environment variable TR_upgradeToNopGuardDuringStartup.

A new query VMEnv::isVMInStartupPhase(TR::Compilation*) is defined to
detect this startup phase. It is intended for downstream projects to
override. The default implementation arbitrarily returns false.

There was pre-existing code that attempted to change profiled guards
into nop guards, but it worked only for VFT tests, and only when the
cold call was a virtual call (although part of the code appeared to be
trying to handle interface calls). The new implementation also works for
method tests and for interface calls, and in the case of method test,
the logic that triggers the transformation is largely shared with other
recent improvements to the handling of method test. As such, this commit
deletes the prior implementation of this concept.
@vijaysun-omr

Copy link
Copy Markdown
Contributor

Build break @jdmpapin

@jdmpapin

jdmpapin commented Oct 5, 2022

Copy link
Copy Markdown
Contributor Author

Updated to remove a J9-ism

@vijaysun-omr

Copy link
Copy Markdown
Contributor

Jenkins build all

@vijaysun-omr vijaysun-omr self-assigned this Oct 6, 2022
@vijaysun-omr

Copy link
Copy Markdown
Contributor

I know @jdmpapin is doing some more personal testing, and so I will await what he gets from that before I merge this.

@jdmpapin

jdmpapin commented Oct 6, 2022

Copy link
Copy Markdown
Contributor Author

My testing has passed - this is good to go

@vijaysun-omr
vijaysun-omr merged commit 696f789 into eclipse-omr:master Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants