[jdk17u-dev] RFR: 8276799: Implementation of JEP 422: Linux/RISC-V Port [v7]

[Robbin Ehn]

On Wed, 5 Jul 2023 06:47:55 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>>> > > > I think we should have some more testing with debugs bit. I have some idle hardware at the moment, so I'll start a test cycle as soon as I get time.
>>> > > 
>>> > > 
>>> > > FYI: I have also performed a tier1 test with fastdebug build on unmatched board during the weekend, result looks good.
>>> > 
>>> > 
>>> > I had one failure on visionfive 2 in tier1:
>>> > ```
>>> > java/lang/invoke/VarHandles/VarHandleTestAccessByte.java false VarHandleTestAccessByte
>>> > test VarHandleTestAccessByte.testAccess("VarHandle -> Array", VarHandle -> Array): failure
>>> > java.lang.AssertionError: success weakCompareAndSetRelease byte expected [true] but found [false]
>>> > ```
>>> 
>>> Seen this before: https://bugs.openjdk.org/browse/JDK-8292407 -- those tests are rather flaky on slow platforms. Chances are, even the updated defaults are not good for relatively slow board like VF2. Try to run that test separately and see if it fails?
>> 
>> Yea, I see, thanks. It passes running it simultaneous as tier2.
>
>> > > > > I think we should have some more testing with debugs bit. I have some idle hardware at the moment, so I'll start a test cycle as soon as I get time.
>> > > > 
>> > > > 
>> > > > FYI: I have also performed a tier1 test with fastdebug build on unmatched board during the weekend, result looks good.
>> > > 
>> > > 
>> > > I had one failure on visionfive 2 in tier1:
>> > > ```
>> > > java/lang/invoke/VarHandles/VarHandleTestAccessByte.java false VarHandleTestAccessByte
>> > > test VarHandleTestAccessByte.testAccess("VarHandle -> Array", VarHandle -> Array): failure
>> > > java.lang.AssertionError: success weakCompareAndSetRelease byte expected [true] but found [false]
>> > > ```
>> > 
>> > 
>> > Seen this before: https://bugs.openjdk.org/browse/JDK-8292407 -- those tests are rather flaky on slow platforms. Chances are, even the updated defaults are not good for relatively slow board like VF2. Try to run that test separately and see if it fails?
>> 
>> Yea, I see, thanks. It passes running it simultaneous as tier2.
> 
> tier2 passed, hotspot_all (not all tests run) only the IR match failure.

> @robehn : I think we have already got a fix for the reported IR Matching failure problem. Turns out that it is the test case that needs to be adapted for RISC-V. So luckily this issue should not block us from proceeding with this PR.
> 
> For 17u, the test passes with the following trivial change:
> 
> ```
> diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java
> index 0d3f22fad8f..d9816525fe6 100644
> --- a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java
> +++ b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java
> @@ -55,11 +55,11 @@ public class IRNode {
>  
>      public static final String ALLOC = "(.*precise klass .*\\R((.*(?i:mov|xorl|nop|spill).*|\\s*|.*LGHI.*)\\R)*.*(?i:call,static).*wrapper for: _new_instance_Java" + END;
>      public static final String ALLOC_OF = COMPOSITE_PREFIX + "(.*precise klass .*" + IS_REPLACED + ":.*\\R((.*(?i:mov|xorl|nop|spill).*|\\s*|.*LGHI.*)\\R)*.*(?i:call,static).*wrapper for: _new_instance_Java" + END;
> -    public static final String ALLOC_ARRAY = "(.*precise klass \[L.*\\R((.*(?i:mov|xor|nop|spill).*|\\s*|.*LGHI.*)\\R)*.*(?i:call,static).*wrapper for: _new_array_Java" + END;
> -    public static final String ALLOC_ARRAY_OF = COMPOSITE_PREFIX + "(.*precise klass \[L.*" + IS_REPLACED + ";:.*\\R((.*(?i:mov|xorl|nop|spill).*|\\s*|.*LGHI.*)\\R)*.*(?i:call,static).*wrapper for: _new_array_Java" + END;
> +    public static final String ALLOC_ARRAY = "(.*precise klass \[L.*\\R((.*(?i:mov|xor|nop|spill).*|\\s*|.*(LGHI|li).*)\\R)*.*(?i:call,static).*wrapper for: _new_array_Java" + END;
> +    public static final String ALLOC_ARRAY_OF = COMPOSITE_PREFIX + "(.*precise klass \[L.*" + IS_REPLACED + ";:.*\\R((.*(?i:mov|xorl|nop|spill).*|\\s*|.*i(LGHI|li).*)\\R)*.*(?i:call,static).*wrapper for: _new_array_Java" + END;
>  
> -    public static final String CHECKCAST_ARRAY = "(((?i:cmp|CLFI|CLR).*precise klass \[.*;:|.*(?i:mov|or).*precise klass \[.*;:.*\\R.*(cmp|CMP|CLR))" + END;
> -    public static final String CHECKCAST_ARRAY_OF = COMPOSITE_PREFIX + "(((?i:cmp|CLFI|CLR).*precise klass \[.*" + IS_REPLACED + ";:|.*(?i:mov|or).*precise klass \[.*" + IS_REPLACED + ";:.*\\R.*(cmp|CMP|CLR))" + END;
> +    public static final String CHECKCAST_ARRAY = "(((?i:cmp|CLFI|CLR).*precise klass \[.*;:|.*(?i:mov|or|mv).*precise klass \[.*;:.*\\R.*(cmp|CMP|CLR))" + END;
> +    public static final String CHECKCAST_ARRAY_OF = COMPOSITE_PREFIX + "(((?i:cmp|CLFI|CLR).*precise klass \[.*" + IS_REPLACED + ";:|.*(?i:mov|or|mv).*precise klass \[.*" + IS_REPLACED + ";:.*\\R.*(cmp|CMP|CLR))" + END;
>      // Does not work on s390 (a rule containing this regex will be skipped on s390).
>      public static final String CHECKCAST_ARRAYCOPY = "(.*((?i:call_leaf_nofp,runtime)|CALL,\\s?runtime leaf nofp|BCTRL.*.leaf call).*checkcast_arraycopy.*" + END;
> ```

Great, thanks. Sorry I don't have time to do a full review (OOO).

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

PR Comment: https://git.openjdk.org/jdk17u-dev/pull/1427#issuecomment-1622560978