RFR: 8333891: Method excluded with directive is not compiled after removal of directive [v2]

[Damon Fenacci]

On Sat, 22 Jun 2024 09:30:25 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> A Java method can become non-compilable if there are issues with its compilation or if its compiled version causes problems. Additionally, a method can be marked as non-compilable using a compile command or a compiler directive. Since compiler directives can be updated, a directive that disables a method's compilation can be changed or removed.
>> 
>> Currently, when a Java method is marked as non-compilable, the reason for this status is unknown. If a change in a compiler directive makes the method compilable again, we cannot clear the non-compilable status because we don't know if the directive initially caused the method to become non-compilable.
>> 
>> To resolve the issue two method flags are introduced: `is_c1_exclude` and `is_c2_excluded`. They mean a Java method is excluded from compilation by a directive. With these flags we can find out a Java method has been excluded with a directive. If the directive changes and allows compilation of the method we can detect this and clear the non-compilable status.
>> 
>> As accesses to flags must be race free we have to remove getting a directive from `CompileBroker::compile_method`. We combine two `CompileBroker::compile_method` into one. We also move calculation whether compilation is blocking into `CompileBroker::compile_method_base`. The directive used for that calculation is passed to a compile task, so a compile task does not need to get it again.
>> 
>> A regression test is added.
>> 
>> Tested fastdebug build on Linux AArch64, Linux x86_64 and Windows Server 2019 x86_64:
>> - Tier1/2/3 passed.
>
> Evgeny Astigeevich has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix data race

Changes requested by dfenacci (Committer).

src/hotspot/share/compiler/compileBroker.cpp line 1336:

> 1334:   }
> 1335: 
> 1336:   AbstractCompiler *comp = CompileBroker::compiler(comp_level);

Do we need to change this?

src/hotspot/share/compiler/compileBroker.cpp line 1563:

> 1561: // See if this compilation is not allowed.
> 1562: bool CompileBroker::compilation_is_prohibited(const methodHandle& method, int osr_bci, int comp_level) {
> 1563:   assert(compiler(comp_level) != nullptr, "Ensure we have a compiler");

Are you sure `compiler(comp_level)` can never be `nullptr` (not even potentially)? (it seems so to me too but just to make sure)

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

PR Review: https://git.openjdk.org/jdk/pull/19637#pullrequestreview-2148291624
PR Review Comment: https://git.openjdk.org/jdk/pull/19637#discussion_r1658904857
PR Review Comment: https://git.openjdk.org/jdk/pull/19637#discussion_r1660722626