JDK-8263024 : Convert Valhalla tests using the old framework to the new framework
  • Type: Sub-task
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: repo-valhalla
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2021-03-04
  • Updated: 2021-08-19
  • Resolved: 2021-07-15
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
repo-valhallaFixed
Related Reports
Relates :  
Description
Starting point from current TestFramework branch: https://github.com/chhagedorn/valhalla/tree/TestingFramework

Goal
===
Convert all tests found in https://github.com/openjdk/valhalla/tree/lworld/test/hotspot/jtreg/compiler/valhalla/inlinetypes that use the old test framework (search for @Test annotation) to use the new test framework. The goal is to have all the converted tests in the same directory again such that we have a diff view of the applied changes. 

I temporarly moved all unprocessed tests from compiler/valhalla/inlinetypes to compiler/valhalla/old_inlinetypes to have a better overview of the ongoing conversion (can be done differently/undone if better suitable). To move files around, use `git mv` as sometimes, moving it directly with `mv` results in a deletion of the old file and creation of a new file and the "applied changes view" is lost).

Requirements
=========
- Remove all references to the old framework and only use the new test framework
- Do not modify the current test code and associated helper methods if it is not absolutely required by the framework (verifiers, tests, helper methods/classes etc.)
- All tests should pass by invoking them with jtreg

Conversion
=======
Maybe the conversion can be automated to some extent. I already moved all helper classes (MyValue1.java, MyValue2.java etc.) to a single file InlineTypes.java such that a JTreg test only needs to specify "@compile InlineTypes.java" if such a helper class is used.

Here is a list of conversion rules that can be applied for the tests:
- Turn someTest_verifier methods into @Run methods
  * @DontCompile of verifiers -> @Run
  * boolean warmup -> TestInfo info, info.isWarmUp()
  * @Warmup at @Test -> @Warmup at @Run
  * @Test(valid = .., failOn = .., match = .., matchCount ..) -> @IR
    - valid -> applyIf
    - regex like STORE, LOAD etc. -> use InlineTypes.STORE, InlineTypes.LOAD etc. or static import InlineTypes.IRNode and directly use STORE, LOAD etc. (all regexes moved to InlineTypes.IRNode)
    - concatted failOn: failOn = STORE + LOAD -> failOn = {InlineTypes.IRNode.STORE, InlineTypes..IRNodeLOAD} (use list and not concat with +)
    - match = {STORE}, matchCount = {1} -> counts = {InlineTypes.IRNode.STORE, "1"} (match/matchCount are now merged together with a single counts attribute with the node regex followed by the count as string)
- Use JTreg header as shown in the applied examples below
- static final String[][] scenarios = {{}, {"-XX:+PatchALot"}} -> use TestFramework.runWithScenarios() and make sure that scenario indices match, starting at 0; remove getNumScenarios() and getVMParameters()
  * Example: ExampleTestUnloadedInlineTypeField (see below)
- getExtraVMParameters -> load default scenarios and add the additionally specified flags, remove getExtraVMParameters()
  * Example: ExampleTestNullableInlineTypes.java
- @OSRCompileOnly -> @Test(compLevel = CompLevel.WAIT_FOR_COMPILATION)
  * Example: jdk.test.lib.hotspot.ir_framework.tests.TestRunTests.test9(): conversion example of  TestOnStackReplacement.test6()
    
Some applied examples of real Valhalla tests:
- ExampleTestNullableInlineTypes.java (based on TestNullableInlineTypes.java):
https://github.com/chhagedorn/valhalla/blob/9ec9f28d715f3566e10252a3db547ed2fd9e0bfb/test/hotspot/jtreg/compiler/valhalla/inlinetypes/ExampleTestUnloadedInlineTypeField.java
- ExampleTestUnloadedInlineTypeField.java (based on TestUnloadedInlineTypeField.java):
https://github.com/chhagedorn/valhalla/blob/9ec9f28d715f3566e10252a3db547ed2fd9e0bfb/test/hotspot/jtreg/compiler/valhalla/inlinetypes/ExampleTestUnloadedInlineTypeField.java

Some made up examples to illustrate the list of conversion rules above
1) Simplest case
  @Test
  public void test(int x) {
  }
- @DontCompile
- public void test_verifier(boolean warmup) {
-     test(34);
- }
+ @Run(test = "test")
+ public void test_verifier(/* TestInfo info */) { // Do not need to specify TestInfo info if warmup not used. But can also keep parameter for more straight forward conversion
+     test(34);
+ }

2) Warmup and query warmup parameter
  @Test
- @Warmup(1000) // For @Run tests: must move @Warmup to @Run method.
  public void test(int x) {
  }
- @DontCompile
- public void test_verifier(boolean warmup) {
-     // Different ways of calling test()
-     if (warmup) {
-         test(someComplexSetup()); // Complex setup of argument
-     } else {
-         verify(test(45));
-     }
- }
+ @Run(test = "test")
+ @Warmup(1000)
+ public void test_verifier(TestInfo info) {
+     if (info.isWarmUp()) {
+         test(someComplexSetup());
+     } else {
+         verify(test(45));
+     }
+ }

3) IR rules
- @Test(valid = ACmpProfileOn, failOn = LOOP + STORE, match = { TRAP }, matchCount = { 1})
- @Test(valid = ACmpProfileOff, failOn = STORE)
- @Test(valid = TypeProfileOn, failOn = LOAD)
- @Test(match = { SUBSTITUTABILITY_TEST }, matchCount = { 1 })
- public void test() {
- }
- @DontCompile
- public void test_verifier(boolean warmup) {
-     test(34);
- }

+ @Test
+ @IR(applyIf = {"UseACmpProfile", "true"}, failOn = {InlineType.IRNode.LOOP, InlineType.IRNode.STORE} , counts = {InlineType.IRNode.TRAP, "1"}
+ @IR(applyIf = {"UseACmpProfile", "false"}, failOn = InlineType.IRNode.STORE)
+ @IR(applyIf = {"TypeProfileLevel", "222"}, failOn = InlineType.IRNode.LOAD)
+ @IR(counts = {InlineType.IRNode.SUBSTITUTABILITY_TEST, "1"})
+ public void test() {
+ }

+ @Run(test = "test")
+ public void test_verifier(/* TestInfo info */) { // Do not need to specify TestInfo info if warmup not used. But can also keep parameter for more straight forward conversion
+     test(34);
+ }


Background Information
===============
Example tests showing the different usages of the framework:
- @Test: https://github.com/chhagedorn/valhalla/blob/TestingFramework/test/lib/jdk/test/lib/hotspot/ir_framework/examples/TestExample.java   
- @Check: https://github.com/chhagedorn/valhalla/blob/TestingFramework/test/lib/jdk/test/lib/hotspot/ir_framework/examples/CheckExample.java
- @Run: https://github.com/chhagedorn/valhalla/blob/TestingFramework/test/lib/jdk/test/lib/hotspot/ir_framework/examplesRunExample.java

Internal tests about different usages:
- Different ways to call the framework (including scenarios and helper classes): https://github.com/chhagedorn/valhalla/blob/TestingFramework/test/lib/jdk/test/lib/hotspot/ir_framework/tests/TestSanity.java
- Other basic tests: https://github.com/chhagedorn/valhalla/blob/TestingFramework/test/lib/jdk/test/lib/hotspot/ir_framework/tests/TestBasics.java

IR Matching:
https://github.com/chhagedorn/valhalla/blob/TestingFramework/test/lib/jdk/test/lib/hotspot/ir_framework/tests/TestIRMatching.java

Forbidden test format:
https://github.com/chhagedorn/valhalla/blob/TestingFramework/test/lib/jdk/test/lib/hotspot/ir_framework/tests/TestBadFormat.java

Other internal tests:
https://github.com/chhagedorn/valhalla/tree/TestingFramework/test/lib/jdk/test/lib/hotspot/ir_framework/tests

More documentation will follow, especially in form of Javadocs and more example tests: JDK-8263026.

Comments
All tests from category 2b have been converted to new IR framework.
14-04-2021

I looked at all compiler/valhalla/inlinetypes tests and they could be categorized this way. ================ (1) Regression tests ================ These tests don't depend on Valhalla/IR framework and no need to be converted. Some small refactoring could be still desired. Add package, .... * TestDeadAllocationRemoval.java // Xbatch * TestFlatArrayAliasesCardMark.java // Xbatch * TestUnresolvedInlineClass.java // delete class and checks, no IR framework needed * TestUnresolvedDefault.java * TestUnloadedInlineTypeArray.java No Changes needed * TestArrayCopyWithOops.java * TestBimorphicInlining.java * TestGenerated.java * TestIsSubstitutableReresolution.java * TestNativeClone.java * TestNestmateAccess.java * TestNewAcmp.java * TestOptimizeKlassCmp.java * TestStressReturnBuffering.java ================================== (2) Valhalla framework based tests ================================== (a) * TestArrayAccessDeopt.java Need to keep as it is, the test is checking deopt output (confirmed by Tobias) * TestBufferTearing.java Does Unsafe checks in multiple concurrent threads. Looks like it is better to leave as it is now. * TestC2CCalls.java Ues -Xbatch, WhiteBox is used to do the same STRESS_SS (-DStressCC) does. * TestDeoptimizationWhenBuffering.java -Xbatch in 7 of 8 scenarious. WhiteBox API is used to disable C2 compilation only in one scenario (-Xbatch is not used) * TestFlatArrayThreshold.java 3 configurations, all with -Xbatch. (b) Tests which extends InlineTypeTest ------------------------------------- These tests should be converted to use new IR framework. + TestArrays.java + TestBasicFunctionality.java + TestC1.java + TestCallingConventionC1.java + TestCallingConvention.java + TestGetfieldChains.java + TestIntrinsics.java + TestJNICalls.java + TestMethodHandles.java + TestNullableArrays.java - TestNullableInlineTypes.java + TestOnStackReplacement.java + TestWithfieldC1.java + TestLWorld.java + TestLWorldProfiling.java + TestUnloadedInlineTypeField.java The tests which are marked by '+' were successfully converted and integrated into Christian's branch. This is slightly more than 75% based on total number of tests annotated by @Test.
13-04-2021