On 8/01/2021 4:09 pm, Igor Veresov wrote:
On Thu, 7 Jan 2021 21:49:56 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
Igor Veresov has updated the pull request incrementally with one additional
commit since the last revision:
Fix s390 build
Marking as reviewed, but only for the jvmti tests. I don't believe there are
any other svc changes in this PR.
Please find the answers in-line.
_Mailing list message from [David Holmes](mailto:david.hol...@oracle.com) on
[shenandoah-dev](mailto:shenandoah-...@openjdk.java.net):_
Hi Igor,
On 8/01/2021 6:36 am, Igor Veresov wrote:
This change removes the legacy compilation policy and an emulation mode to the
tiered policy to simulate the old behavior with ```-XX:-TieredCompilation```.
The change removed a bunch of interpreter code, devirtualizes the compilation
policy API, adds a consistent way to query compiler configuration with the new
```CompilerConfig``` API.
Can you clarify, for non-compiler folk, what all the alternative configs
actually mean now. I'm a bit confused by this definition:
define_pd_global(bool, TieredCompilation,
COMPILER1_PRESENT(true) NOT_COMPILER1(false));
That's in a c2_globals_*.hpp, which is only included if C2 is present. Given
that, if C1 is present as well the flag is set to true.
Sorry I overlooked the exact placement.
as I expected tiered compilation to require COMPILER1 and COMPILER2.
Right. That's exactly what's happening.
Also I see interpreter code that used to be guarded by TieredCompilation
now being executed unconditionally, which seems to assume C1 or C2 must
be present?
Counters and compilation policy callbacks are historically guarded by
UseCompiler and UseLoopCounter flags, which are set to false if there are no
compilers present. I haven't changed the semantics.
That is not at all obvious. For example in
templateInterpreterGenerator_aarch64.cpp this code used to guarded by
TieredCompilation:
608: __ bind(no_mdo);
// Increment counter in MethodCounters
const Address invocation_counter(rscratch2,
MethodCounters::invocation_counter_offset() +
InvocationCounter::counter_offset());
__ get_method_counters(rmethod, rscratch2, done);
const Address mask(rscratch2,
in_bytes(MethodCounters::invoke_mask_offset()));
__ increment_mask_and_jump(invocation_counter, increment, mask,
rscratch1, r1, false, Assembler::EQ, overflow);
__ bind(done);
but now it seems to be executed unconditionally with no reference to
either flag you mentioned.
Overall it is a big change to digest, but after skimming it looks like a
few of the refactorings could have been applied in a layered fashion
using multiple commits to make it easier to review.
Frankly, I don't know how to split it into smaller pieces. There are
surprisingly lots of interdependencies. However, the best way to think about
it is that there are three major parts: 1. CompilerConfig API that is an
abstraction over compiler configuration (what's compiled in, what flags are
present that restrict compiler usage, etc); 2. The legacy policy removal. 3.
Emulation of the old JVM behavior.
I was thinking the ifdef related changes as one commit; then the
TieredCompilation removals; then the addition of CompilerConfig API ...
These aren't separate changes just different commits within the PR so
that it can be reviewed in stages.
Cheers,
David
-----
As far as the runtime part of the change is concerted, it's pretty much only
the legacy policy removal. In particular, the parts of the interpreter that
dealt with the old policy (basically everything in the interpreter that was
under !TieredCompilation has gotten removed).
Igor
Thanks,
David
-----
-------------
PR: https://git.openjdk.java.net/jdk/pull/1985
