RFR: 8254129: IR Test Framework to support regex-based matching on the IR in JTreg compiler tests [v5]

Christian Hagedorn chagedorn at openjdk.java.net
Fri Apr 23 12:39:33 UTC 2021


On Thu, 22 Apr 2021 13:30:17 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

> Great work, Christian! I'm glad we finally have the capability to write regression tests with IR verification.
> 
> Hard to review this in detail but I've added some comments. In any case, conversion of Valhalla tests to the new framework has proven that this is good to go. We can still fix problems and add new features once this is in.

Thank you Tobias for your review! I pushed an update with your review comments. I agree that we can still follow up with fixes and new features once this went it.

> test/lib/jdk/test/lib/hotspot/ir_framework/Check.java line 94:
> 
>> 92: @Retention(RetentionPolicy.RUNTIME)
>> 93: public @interface Check {
>> 94: 
> 
> Some classes/interfaces have a newline at the beginning and others don't. I think they can be removed.

Cleaned that up together with some other formatting.

> test/lib/jdk/test/lib/hotspot/ir_framework/README.md line 128:
> 
>> 126: This framework is based on the idea of the currently present IR test framework in [Valhalla](https://github.com/openjdk/valhalla/blob/e9c78ce4fcfd01361c35883e0d68f9ae5a80d079/test/hotspot/jtreg/runtime/valhalla/inlinetypes/InlineTypesTest.java). This IR framework was used with great success in Valhalla and thus served as a foundation for this new IR framework.
>> 127:  
>> 128:  The new framework supports all the features that are present in the Valhalla IR framework with the idea to replace it at some point. The initial design and feature set was kept simple and straight forward and serves well for small to medium sized tests. There are a lot of possibilities to further enhance the framework and make it more powerful. This can be tackled in additional RFEs. A few ideas include:
> 
> Again, I would remove the reference to the Valhalla framework. Looking through the changes, I see 13 references to "Valhalla", I think these should all be removed.

I removed all references except the ones in the testing section about the converted tests. I think these references are important to keep for now.

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

PR: https://git.openjdk.java.net/jdk/pull/3508


More information about the hotspot-compiler-dev mailing list