RFR(S): 8235673: [C1, C2] Split inlining control flags

[Vladimir Kozlov]

I would suggest to build VM without C2 and run tests.

I grepped tests with these flags I found next tests where we need to fix test's command (add 
-XX:+IgnoreUnrecognizedVMOptions) or add  @requires vm.compiler2.enabled or duplicate test for C1 with corresponding C1 
flags (by ussing additional @test block).

runtime/ReservedStack/ReservedStackTest.java
compiler/intrinsics/string/TestStringIntrinsics2.java
compiler/c2/Test6792161.java
compiler/c2/Test5091921.java

And there is issue with compiler/compilercontrol tests which use InlineSmallCode and I am not sure how to handle:

http://hg.openjdk.java.net/jdk/jdk/file/55e9cb6b23ec/test/hotspot/jtreg/compiler/compilercontrol/share/scenario/Command.java#l36

Thanks,
Vladimir

On 5/4/20 9:04 AM, Doerr, Martin wrote:
> Hi Nils,
> 
> thank you for looking at this and sorry for the late reply.
> 
> I've added MaxTrivialSize and also updated the issue accordingly. Makes sense.
> Do you have more flags in mind?
> 
> Moving the flags which are only used by C2 into c2_globals definitely makes sense.
> 
> Done in webrev.01:
> http://cr.openjdk.java.net/~mdoerr/8235673_C1_inlining/webrev.01/
> 
> Please take a look and let me know when my proposal is ready for a CSR.
> 
> Best regards,
> Martin
> 
> 
>> -----Original Message-----
>> From: hotspot-compiler-dev <hotspot-compiler-dev-
>> bounces at openjdk.java.net> On Behalf Of Nils Eliasson
>> Sent: Dienstag, 28. April 2020 18:29
>> To: hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: RFR(S): 8235673: [C1, C2] Split inlining control flags
>>
>> Hi,
>>
>> Thanks for addressing this! This has been an annoyance for a long time.
>>
>> Have you though about including other flags - like MaxTrivialSize?
>> MaxInlineSize is tested against it.
>>
>> Also - you should move the flags that are now c2-only to c2_globals.hpp.
>>
>> Best regards,
>> Nils Eliasson
>>
>> On 2020-04-27 15:06, Doerr, Martin wrote:
>>> Hi,
>>>
>>> while tuning inlining parameters for C2 compiler with JDK-8234863 we had
>> discussed impact on C1.
>>> I still think it's bad to share them between both compilers. We may want to
>> do further C2 tuning without negative impact on C1 in the future.
>>>
>>> C1 has issues with substantial inlining because of the lack of uncommon
>> traps. When C1 inlines a lot, stack frames may get large and code cache space
>> may get wasted for cold or even never executed code. The situation gets
>> worse when many patching stubs get used for such code.
>>>
>>> I had opened the following issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8235673
>>>
>>> And my initial proposal is here:
>>> http://cr.openjdk.java.net/~mdoerr/8235673_C1_inlining/webrev.00/
>>>
>>>
>>> Part of my proposal is to add an additional flag which I called
>> C1InlineStackLimit to reduce stack utilization for C1 methods.
>>> I have a simple example which shows wasted stack space (java example
>> TestStack at the end).
>>>
>>> It simply counts stack frames until a stack overflow occurs. With the current
>> implementation, only 1283 frames fit on the stack because the never
>> executed method bogus_test with local variables gets inlined.
>>> Reduced C1InlineStackLimit avoids inlining of bogus_test and we get 2310
>> frames until stack overflow. (I only used C1 for this example. Can be
>> reproduced as shown below.)
>>>
>>> I didn't notice any performance regression even with the aggressive setting
>> of C1InlineStackLimit=5 with TieredCompilation.
>>>
>>> I know that I'll need a CSR for this change, but I'd like to get feedback in
>> general and feedback about the flag names before creating a CSR.
>>> I'd also be glad about feedback regarding the performance impact.
>>>
>>> Best regards,
>>> Martin
>>>
>>>
>>>
>>> Command line:
>>> jdk/bin/java -XX:TieredStopAtLevel=1 -XX:C1InlineStackLimit=20 -
>> XX:C1MaxRecursiveInlineLevel=0 -Xss256k -Xbatch -XX:+PrintInlining -
>> XX:CompileCommand=compileonly,TestStack::triggerStackOverflow
>> TestStack
>>> CompileCommand: compileonly TestStack.triggerStackOverflow
>>>                                 @ 8   TestStack::triggerStackOverflow (15 bytes)   recursive
>> inlining too deep
>>>                                 @ 11   TestStack::bogus_test (33 bytes)   inline
>>> caught java.lang.StackOverflowError
>>> 1283 activations were on stack, sum = 0
>>>
>>> jdk/bin/java -XX:TieredStopAtLevel=1 -XX:C1InlineStackLimit=10 -
>> XX:C1MaxRecursiveInlineLevel=0 -Xss256k -Xbatch -XX:+PrintInlining -
>> XX:CompileCommand=compileonly,TestStack::triggerStackOverflow
>> TestStack
>>> CompileCommand: compileonly TestStack.triggerStackOverflow
>>>                                 @ 8   TestStack::triggerStackOverflow (15 bytes)   recursive
>> inlining too deep
>>>                                 @ 11   TestStack::bogus_test (33 bytes)   callee uses too
>> much stack
>>> caught java.lang.StackOverflowError
>>> 2310 activations were on stack, sum = 0
>>>
>>>
>>> TestStack.java:
>>> public class TestStack {
>>>
>>>       static long cnt = 0,
>>>                   sum = 0;
>>>
>>>       public static void bogus_test() {
>>>           long c1 = 1, c2 = 2, c3 = 3, c4 = 4;
>>>           sum += c1 + c2 + c3 + c4;
>>>       }
>>>
>>>       public static void triggerStackOverflow() {
>>>           cnt++;
>>>           triggerStackOverflow();
>>>           bogus_test();
>>>       }
>>>
>>>
>>>       public static void main(String args[]) {
>>>           try {
>>>               triggerStackOverflow();
>>>           } catch (StackOverflowError e) {
>>>               System.out.println("caught " + e);
>>>           }
>>>           System.out.println(cnt + " activations were on stack, sum = " + sum);
>>>       }
>>> }
>>>
>