RFR: 8270086: ARM32-softfp: Do not load CONSTANT_double using the condy helper methods in the interpreter

[Christoph Göttschkes]

On Tue, 13 Jul 2021 13:17:58 GMT, Christoph Göttschkes <cgo at openjdk.org> wrote:

> Hi,
> 
> please review this fix for the template interpreter for ARM32-softfp.
> 
> With the introduction of condy, the interpreter started to load double constants using the condy_helper, which is unnecessary. Also, the [resolve_ldc](https://github.com/openjdk/jdk/blob/8973867fb9568a3a527b763c9ce10cebdfb306d0/src/hotspot/share/interpreter/interpreterRuntime.cpp#L148) implementation of the interpreter runtime might not be designed to load constant values. It should only load strings and condy.
> 
> https://github.com/openjdk/jdk/blob/2ee56fd1cf4acd634e19cc77b7c9a6858e1c315a/src/hotspot/cpu/arm/templateTable_arm.cpp#L515-L560
> 
> I refactored the ldc2_w bytecode imlpementation. Now, the ifdef between soft and hard float covers both types, CONSTANT_Long and CONSTANT_Double. I did this, because for the softfp, we can use a conditional cmp, since double and long are both placed on the stack in the same way. The same is also done for CONSTANT_Integer and CONSTANT_Float in the ldc bytecode implementation. Also, I think it is easier to follow the code this way.
> 
> Old ldc2_w implementation (before condy was implemented for ARM32):
> 
> https://github.com/openjdk/jdk/blob/7101b28dc3903be27cb46a00781f4d74f81f1114/src/hotspot/cpu/arm/templateTable_arm.cpp#L497-L537
> 
> Testing: ARMv7-A (hardfp) hotspot tier1, ARMv5TE (softfp) hotspot tier1

Thanks for looking into this, Aleksey.

I will look into your suggestions. The reason I did a bigger #ifdef block was, because I think the problem was introduced because the softfp path was mixed into the hardfp one. For me, this makes the softfp path harder to understand, in contrast to my suggested solution. I know that having bigger #ifdef brings other problems with it. Do you think it is better to keep the #ifdef as small as possible and mix the softfp path into the hardfp?

> TBH, I am not even sure that we need to check __ABI_HARD__ here instead of __SOFTFP__ here. Looking at TemplateTable::dload and load_category2_local, I would have expected __SOFTFP__...

I don't quite understand what you mean here. Both, ABI_HARD and SOFTFP should be macros which specify which ABI should be used, so for me, if `__SOFTFP__` is `1`, `__ABI_HARD__` is `0`, or the other way around, or am I missing something? Could you elaborate why you think this should be changed?

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

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