RFR: 8366118: DontCompileHugeMethods is not respected with -XX:-TieredCompilation [v3]

[Man Cao]

On Mon, 25 Aug 2025 22:35:47 GMT, Man Cao <manc at openjdk.org> wrote:

>> src/hotspot/share/compiler/compilationPolicy.cpp line 925:
>> 
>>> 923:   }
>>> 924: 
>>> 925:   if (!CompilationModeFlag::disable_intermediate()) {
>> 
>> AFAICT, the block of code here is intended for handling the case when intermediate is not disabled. Your change subtly alters that. 
>> 
>> When `TieredCompilation` is disabled, the large method compilation is done via  `CompileBroker::compile_method` if `!CompileBroker::compilation_is_in_queue(mh)` is `true`. I confirmed that in lldb, see below. Is there any reason to not do `can_be_compiled` check when calling `CompileBroker::compile_method`?
>> 
>> Additionally, should `can_be_compiled` check only be done for c2 compilation or if it should also be applied to c1 compilation? 
>> 
>> 
>> (lldb) bt
>> * thread #18, name = 'ApexEnumTest_de', stop reason = step in
>>   * frame #0: 0x00007ffff49b70ae libjvm.so`CompileBroker::compile_method(method=0x00007ffff38dba10, osr_bci=-1, comp_level=4, hot_method=0x00007ffff38dba10, hot_count=6784, compile_reason=Reason_Tiered, __the_thread__=0x00001354ff8c9810) at compileBroker.cpp:1347:21
>>     frame #1: 0x00007ffff49981fb libjvm.so`CompilationPolicy::compile(mh=0x00007ffff38dba10, bci=-1, level=CompLevel_full_optimization, __the_thread__=0x00001354ff8c9810) at compilationPolicy.cpp:824:5
>>     frame #2: 0x00007ffff4997baf libjvm.so`CompilationPolicy::method_invocation_event(mh=0x00007ffff38dba10, imh=0x00007ffff38dba10, level=CompLevel_none, nm=0x0000000000000000, __the_thread__=0x00001354ff8c9810) at compilationPolicy.cpp:1160:7
>>     frame #3: 0x00007ffff49979ea libjvm.so`CompilationPolicy::event(method=0x00007ffff38dba10, inlinee=0x00007ffff38dba10, branch_bci=-1, bci=-1, comp_level=CompLevel_none, nm=0x0000000000000000, __the_thread__=0x00001354ff8c9810) at compilationPolicy.cpp:745:5
>>     frame #4: 0x00007ffff4d79dd8 libjvm.so`InterpreterRuntime::frequency_counter_overflow_inner(current=0x00001354ff8c9810, branch_bcp=0x0000000000000000) at interpreterRuntime.cpp:1066:21
>>     frame #5: 0x00007ffff4d79a76 libjvm.so`InterpreterRuntime::frequency_counter_overflow(current=0x00001354ff8c9810, branch_bcp=0x0000000000000000) at interpreterRuntime.cpp:1015:17
>>     frame #6: 0x00007fffe1c0ce41
>>     frame #7: 0x00007fffe1c080a8
>>     frame #8: 0x00007fffe1c00d01
>>     frame #9: 0x00007ffff4d8786d libjvm.so`JavaCalls::call_helper(result=0x00007ffff38dc040, method=0x00007ffff38dbf90, args=0x00007ffff38dbec8, __the_thread__=0x00001354ff8c9810) at javaCalls.cpp:415:7
>>     frame #10...
>
>> AFAICT, the block of code here is intended for handling the case when intermediate is not disabled. Your change subtly alters that.
>> When TieredCompilation is disabled, the large method compilation is done via CompileBroker::compile_method if !CompileBroker::compilation_is_in_queue(mh) is true. I confirmed that in lldb, see below. Is there any reason to not do can_be_compiled check when calling CompileBroker::compile_method?
> 
> Trying to compile the large method under `-XX:-TieredCompilation` is the bug. The large method should not be compiled under `-XX:+DontCompileHugeMethods`.
> 
> The bug is caused by erroneously guarding the `!can_be_compiled()` and `!can_be_osr_compiled()` checks behind `!CompilationModeFlag::disable_intermediate()`. The correct behavior is to do the following checks and returns regardless of `TieredCompilation`:
> 
> if ((bci == InvocationEntryBci && !can_be_compiled(mh, level))) {
>   return;
> }
> if ((bci != InvocationEntryBci && !can_be_osr_compiled(mh, level))) {
>   return;
> }
> 
> Only the recursive call to `compile(mh, bci, CompLevel_simple, THREAD)` and `osr_nm->make_not_entrant()` need to be guarded under `!disable_intermediate()`.
> 
> It is possible to add the above two checks for `bci`, `can_be_compiled()` and `!can_be_osr_compiled()` to inside `CompileBroker::compile_method()`, specifically inside `CompileBroker::compilation_is_prohibited()`. If compiler-dev team prefers this way, we could move them.

To answer more directly:
> Additionally, should can_be_compiled check only be done for c2 compilation or if it should also be applied to c1 compilation?

`can_be_compiled()` and `can_be_osr_compiled()` should be applied to both C1 and C2 compilation.

> Is there any reason to not do `can_be_compiled` check when calling `CompileBroker::compile_method`?

My rationale is to keep the code similar prior to [JDK-8251462](https://bugs.openjdk.org/browse/JDK-8251462). As mentioned above, it is possible to add those checks to `CompileBroker::compile_method()` or `CompileBroker::compilation_is_prohibited()`. I could do that if there's a strong preference. A potential issue with that approach is that the code here in `CompilationPolicy::compile()` is still confusing: why do those two `if (...) { return;}` checks only apply to `!CompilationModeFlag::disable_intermediate()`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26932#discussion_r2305060279