Re: gcc, arm, and thumbs mode

2022-05-29 Thread Thomas Stüfe
Hi Anton,

thanks for looking into this.

For my specific problem, I can and probably should use .function. But my
question was more general, should we leave the decision whether to use
thumb or arm up to the toolchain. For two reasons, one is executable size -
I assume using thumb has certain advantages, executables are smaller and
you can fit more into the instruction cache, and second, because errors
like mine are probably not that uncommon and it makes me nervous that we
only caught it because someone happened to build on his Raspi.

Cheers, Thomas

On Fri, May 27, 2022 at 3:28 PM Anton Kozlov  wrote:

> Hi Thomas,
>
> On 5/27/22 16:12, Thomas Stüfe wrote:
> > P.S. I found one possible solution for my particular problem was to add
> > `.type function` to the static assembler routine. That caused gcc to use
> > the correct jump instruction (blx instead of bl). But I am not sure this
> is
> > the best solution, maybe best would be to just use the same mode for all
> > hotspot compilation units.
>
> AFAIR, that .type %function directive is a correct way to write asm
> code. At least this is what gcc generates for the C code [1]. I'm not
> sure how the annotation in the assembly code affects the caller code,
> may be link time optimization? But if adding the directive resolves the
> issue, I vote for it.
>
> (I expect arm-none crosscompiler to produce similar results compared to
> arm-linux target)
>
> Thanks,
> Anton
>
> $ echo "int main() { return 0; }" | arm-none-eabi-gcc -mthumb -S -x c - -o
> -
> .cpu arm7tdmi
> .arch armv4t
> .fpu softvfp
> .eabi_attribute 20, 1
> .eabi_attribute 21, 1
> .eabi_attribute 23, 3
> .eabi_attribute 24, 1
> .eabi_attribute 25, 1
> .eabi_attribute 26, 1
> .eabi_attribute 30, 6
> .eabi_attribute 34, 0
> .eabi_attribute 18, 4
> .file   ""
> .text
> .align  1
> .global main
> .syntax unified
> .code   16
> .thumb_func
> .type   main, %function
> main:
> @ Function supports interworking.
> @ args = 0, pretend = 0, frame = 0
> @ frame_needed = 1, uses_anonymous_args = 0
> push{r7, lr}
> add r7, sp, #0
> movsr3, #0
> movsr0, r3
> mov sp, r7
> @ sp needed
> pop {r7}
> pop {r1}
> bx  r1
> .size   main, .-main
> .ident  "GCC: (Arch Repository) 12.1.0"
>
>


Re: RFR: 8287171: Refactor null caller tests to a single directory [v3]

2022-05-29 Thread Alan Bateman
On Mon, 30 May 2022 00:10:50 GMT, Tim Prinzing  wrote:

>> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates 
>> a test module with some resources in it for the actual tests that occur at 
>> the native level. The native part was switched to c++ instead of c to make 
>> it easier to create helper objects that reduce the redundant code performing 
>> error checking. The two helper classes InstanceCall and StaticCall were 
>> placed in an include file called CallHelper.hpp. The main part of the tests 
>> are in exeNullCallerTest.cpp, and there is a separate function for each of 
>> the bug reports.
>
> Tim Prinzing has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   problem with iostream on Windows, use printf

I don't think jdk/nullCaller is right location for this. Maybe jni/nullCaller 
could work. You'll probably need to add the location to an appropriate 
group/tier in test/jdk/TEST.groups, otherwise it won't be run.

-

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


Re: RFR: 8287171: Refactor null caller tests to a single directory [v3]

2022-05-29 Thread Tim Prinzing
> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates 
> a test module with some resources in it for the actual tests that occur at 
> the native level. The native part was switched to c++ instead of c to make it 
> easier to create helper objects that reduce the redundant code performing 
> error checking. The two helper classes InstanceCall and StaticCall were 
> placed in an include file called CallHelper.hpp. The main part of the tests 
> are in exeNullCallerTest.cpp, and there is a separate function for each of 
> the bug reports.

Tim Prinzing has updated the pull request incrementally with one additional 
commit since the last revision:

  problem with iostream on Windows, use printf

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8934/files
  - new: https://git.openjdk.java.net/jdk/pull/8934/files/a54aa747..f1406a45

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8934=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8934=01-02

  Stats: 14 lines in 2 files changed: 8 ins; 2 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8934.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8934/head:pull/8934

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


Re: RFR: 8287171: Refactor null caller tests to a single directory [v2]

2022-05-29 Thread Tim Prinzing
> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates 
> a test module with some resources in it for the actual tests that occur at 
> the native level. The native part was switched to c++ instead of c to make it 
> easier to create helper objects that reduce the redundant code performing 
> error checking. The two helper classes InstanceCall and StaticCall were 
> placed in an include file called CallHelper.hpp. The main part of the tests 
> are in exeNullCallerTest.cpp, and there is a separate function for each of 
> the bug reports.

Tim Prinzing has updated the pull request with a new target base due to a merge 
or a rebase. The incremental webrev excludes the unrelated changes brought in 
by the merge/rebase. The pull request contains two additional commits since the 
last revision:

 - Merge branch 'master' into JDK-8287171
 - 8287171: Refactor null caller tests to a single directory

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8934/files
  - new: https://git.openjdk.java.net/jdk/pull/8934/files/5eba965b..a54aa747

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8934=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8934=00-01

  Stats: 2803 lines in 65 files changed: 1631 ins; 617 del; 555 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8934.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8934/head:pull/8934

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


Re: RFR: 8287366: Improve test failure reporting in GHA

2022-05-29 Thread Christoph Langer
On Thu, 26 May 2022 12:04:41 GMT, Magnus Ihse Bursie  wrote:

> It is currently both tricky and tedious to figure out what went wrong when a 
> jtreg test fails in GHA.
> 
> We should utilize the full potential of GitHub Action summaries and error 
> annotations to make finding failures easier and more discoverable.
> 
> With this PR, the overview of failures are presented on the "Summary" page 
> for the action (the top-most line to the left, with the outline house icon). 
> Below the `submit.yml` dependency graph, you'll find the annotations, which 
> will look like this:
> 
> 
> Linux x86 (jdk/tier1 part 1)
> Test run reported 34 test failure(s) and 0 error(s). See summary for details.
> 
> 
> Below the annotations follow the summaries. Go have a look at the runs for 
> this PR to see what it looks like! In short, there is a separate summary per 
> test job. The first part lists the names of the failed tests. This will 
> always be included. Below this (with links from the summary list) are 
> detailed information for each failed test. This include the jtreg output, and 
> the `hs_err` file(s), if present. The latter part has a limit from Github on 
> 1 MB. If this limit is broken, no detailed information at all is presented 
> (sorry 'bout that; GitHub's rules).
> 
> This PR is deliberately based on a commit prior to the fix for JDK-8287137 
> (Problemlist failing x86_32 tests after Loom integration), so you can see for 
> yourself how the GHA runs looks in case of a "train wreck" testing situation, 
> like on x86 after Loom. As you can see, most of the output part of the 
> summaries got larger than the 1 MB limit, which means they were not shown. 
> Only the summary for `Linux x86 (hs/tier1 runtime)` displays as intended. 
> OTOH, this shows that the system has a "graceful degradation" mode for even 
> large amount of failures like this. And, since I don't see a Loom v2.0 coming 
> anytime soon, I believe this amount of failed tests are unlikely to be a 
> realistic scenario.
> 
> Finally: the duplication in submit.yml is really, really annoying. :-( I have 
> copied the same code block to three places. The fourth place, for Windows, do 
> not get any support at this time. Concurrently with this change, I have 
> started a separate branch where I split up submit.yml into reusable parts, 
> using "callable workflows" and "custom actions". As part of this effort, I 
> will also change the windows jobs to use cygwin bash instead of PowerShell. 
> Until then, I could not be bothered to even think about implementing this 
> functionality in PS. When that change is integrated, Windows will get this 
> functionality for free, too.

This is a great improvement to GHA. I'm also looking forward to your 
de-duplication efforts and basing the windows steps on cygwin to benefit from 
the error handling there as well. Thanks for doing this!

-

Marked as reviewed by clanger (Reviewer).

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