Advice on how to push changes back for ARMv7-A softfp platforms

[Boris Ulasevich]

Hi Christoph,

   I see. I do not know how to write test for the issue too. By the way, 
current implementation can (1) make unnecessary call to interpreter 
runtime and (2) can make wrong load when Rtemp is equal to 
JVM_CONSTANT_Long by chance. So the change is critical for arm32-sflt.

   Did you run jtreg tests?

   Please proceed with review. I can update webrev and bug report and 
help you with pushing the change when it is accepted (I am not a reviewer).

regards,
Boris

On 09.08.2019 15:49, christoph.goettschkes at microdoc.com wrote:
> Hi Boris,
> 
> thank you for the webrev.
> 
> I checked jdk/jdk and you are right, there are differences in the file,
> but not regarding this issue. Some of the differences are because the
> AArch64 code has been removed from the cpu/arm port in jdk/jdk. In
> jdk-updates/jdk11u, the cpu/arm source base also includes an AArch64 port.
> The patch looks good to me.
> 
> I did not create a test case for this issue, because I don't know if one
> is able to check if the template interpreter calls the interpreter runtime
> or not.
> Resolving this issue makes the template interpreter faster, because it
> does not call the interpreter runtime (using the condy_helper function),
> but is able to load in the constant in the generated code. A simple test
> case (which I used and checked using a debugger, back when I patched our
> source base) is something like the following (It is not the test I used,
> but should work as well):
> 
> class Test {
>      public static void main(String... args) {
>          long x = 12345678910L;
>      }
> }
> 
> Also, since the Rtemp is not initialized in the code generated by this
> template, it could be, that the path for loading a long constant is used,
> even if the type is of double, but I don't know if this is only a
> theoretical bug or if it might appear in practice as well. I never saw it
> happening.
> 
> Yes, there is an OCA signed for MicroDoc Software GmbH.
> 
> Thanks, Christoph
> 
> Boris Ulasevich <boris.ulasevich at bell-sw.com> wrote on 2019-08-09
> 13:49:00:
> 
>> From: Boris Ulasevich <boris.ulasevich at bell-sw.com>
>> To: christoph.goettschkes at microdoc.com
>> Cc: hotspot-dev at openjdk.java.net
>> Date: 2019-08-09 13:49
>> Subject: Re: Advice on how to push changes back for ARMv7-A softfp
> platforms
>>
>> Hi Christoph,
>>
>> I can create webrev for you. Please note that we should not directly
>> patch jdk11u. We should push the fix to jdk-jdk, and then backport the
>> change to jdk11u. Your patch is not applied cleanly to the upstream
>> because of a minor change around exit label. I think for jdk-jdk the
>> change can be like this:
>> http://cr.openjdk.java.net/~bulasevich/8229352/webrev.00
>>
>> A good practice for review is to add a test to reproduce the problem.
>>
>> Did your company (or you personally) signed OCA?
>>
>> thanks,
>> Boris
>>
>