RFR: 8239423: [TESTBUG] jdk/jfr/jvm/TestJFRIntrinsic.java failed with -XX:-TieredCompilation

[David Holmes]

Hi Igor,

On 25/02/2020 2:42 am, Igor Ignatyev wrote:
> Hi
> 
>> I found it's hard to find all the tests that use the modified
>> functions, because not all the tests use the modified functions
>> directly.
> 
> I'd just change the functions in question to crash JVM, and then all the 
> tests which start to fail are the tests which use these functions 
> directly or indirectly. that's quite similar to your 2nd way.
> I'll be very surprised if there are any non hotspot tests (modulo jfr) 
> which use this API, so I'd just run hotspot and jdk/jfr tests.
> 
>> I also tested them with c1-only configuration. 2 failures were
>> found, but regression were not found.
> what are these two failures?
> 
>> Do I need do some other tests?
> applications/ctw tests definitely use the modified methods, so I'd 
> recommend you to run one tests from there 
> (e.g. applications/ctw/modules/java_base.java)
> 
> regarding your patch, 'cl_min' name seems to be confusion (at least to 
> me) as it actually works as an upper bound for valid compilation levels. 

Yes that is my fault. I use cl_min because it captured the result of the 
MIN2 macro. But it is actually the lesser of two possible upper bounds

> also given you had to change the same code in 3 different places, 
> wouldn't it be better to factor that out? so you'll have smth like:
>> + if (compLevel < CompLevel_none || compLevel > highestCompLevel()) {
>>       return false; // Intrinsic is not available on a non-existent compilation level.
>>     }

Yes a file static function would be useful to capture this.

> I'd like to cycle back on David's idea ("I actually think it is a bug 
> that the VM allows you to set TieredStopAtLevel even though it is 
> ignored on those configurations") Isn't the actual problem in how 
> TieredStopAtLevel flag is handled in cases when TieredCompilation is 
> disabled and/or JVM is built w/o tiered compilation support? I mean if 
> JVM ensured that in such cases TieredStopAtLevel is set to 
> CompLevel_highest_tier, all these WB methods would be correct.

TieredStopAtLevel is an input flag. I don't think it makes sense to 
treat it as an output flag, and not only ignore what the user sets but 
replace it with another value when there is no tiered. Ultimately this 
is a design decision of the compiler team, but as it stands any code 
that uses TieredStopAtLevel has to ignore it when there is no tiered.

Cheers,
David
-----

> Thanks,
> -- Igor
> 
>> On Feb 24, 2020, at 3:39 AM, Ao Qi <aoqi at loongson.cn 
>> <mailto:aoqi at loongson.cn>> wrote:
>>
>> On Fri, Feb 21, 2020 at 10:42 AM Ao Qi <aoqi at loongson.cn 
>> <mailto:aoqi at loongson.cn>> wrote:
>>>
>>>> We need to find all tests that use the modified WB functions and see if
>>>> they all pass under normal and non-tiered configurations.
>>
>> I found it's hard to find all the tests that use the modified
>> functions, because not all the tests use the modified functions
>> directly. I try to find them by two ways:
>>  - I grep all modified functions' name in test dir and found all the
>> tests contain the names. Some found files are not jtreg tests. They
>> are used by other tests or libs. I also grep these files' name to find
>> tests that use these files. However I missed the situation that other
>> libs use these files.
>>  - I added some tty->print at the modified positions and run hotspot
>> (hotspot_all_no_apps), jdk (jdk_stable) and langtools (jdk lib tools)
>> tests. Then I found all jtr files which contain the added print.
>> The test list found by the above two ways are here:
>> http://cr.openjdk.java.net/~aoqi/8239423/all.txt . These tests passed
>> with default and -XX:-TieredCompilation parameter and passed under
>> --with-jvm-features=-compiler1 configuration. I also tested them with
>> c1-only configuration. 2 failures were found, but regression were not
>> found.
>>
>> I also tested tier1-3, both release and fastdebug, with default and
>> -XX:-TieredCompilation parameter.
>>
>> Do I need do some other tests?
>>
>> Cheers,
>> Ao Qi
>>
>>
>>
>>
>>
>>
>>
>>
>>>
>>> I agree. I will try to find these tests.
>>>
>>> Cheers,
>>> Ao Qi
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>
>