RFR: aarch32: ARM 32 build broken after 8165929

[David Holmes]

Hi Ed,

Small nit: can you please include the current bug number in the RFR 
email subject. Thanks.

On 16/03/2018 7:40 AM, Edward Nevill wrote:
> Hi,
> 
> Please review the following webrev
> 
> Bugid: https://bugs.openjdk.java.net/browse/JDK-8199243
> Webrev: http://cr.openjdk.java.net/~enevill/8199243/webrev.00
> 
> The ARM 32 build is broken with multiple errors of the form
> 
> /work/ed/arm32/jdk/src/hotspot/os_cpu/linux_arm/copy_linux_arm.inline.hpp:33:54: error: invalid conversion from 'const HeapWord*' to 'HeapWord*' [-fpermissive]
>     _Copy_conjoint_words(to, from, count * HeapWordSize);
> 
> The problem was introduced by change 8165929
> 
> # HG changeset patch
> # User coleenp
> # Date 1518182622 18000
> #      Fri Feb 09 08:23:42 2018 -0500
> # Node ID 950c35ea6237afd834d02345a2878e5dc30750e0
> # Parent  f323537c9b75444578c75d348fa2e5be81532d3e
> 8165929: Constify arguments of Copy methods
> Reviewed-by: hseigel, kbarrett
> 
> This change added 'const' to the 'from' arguments in various Copy functions, for example
> 
> -  void _Copy_conjoint_words(HeapWord* from, HeapWord* to, size_t count);
> -  void _Copy_disjoint_words(HeapWord* from, HeapWord* to, size_t count);
> +  void _Copy_conjoint_words(const HeapWord* from, HeapWord* to, size_t count);
> +  void _Copy_disjoint_words(const HeapWord* from, HeapWord* to, size_t count);
> 
> The problem in the ARM 32 port occurs in code like the following
> 
> static void pd_conjoint_words(const HeapWord* from, HeapWord* to, size_t count) {
> #ifdef AARCH64
>    _Copy_conjoint_words(from, to, count * HeapWordSize);
> #else
>     // NOTE: _Copy_* functions on 32-bit ARM expect "to" and "from" arguments in reversed order
>    _Copy_conjoint_words(to, from, count * HeapWordSize);
> #endif
> }
> 
> The assembler implementation of the Copy functions in ARM 32 actually copies in the wrong order. Ie it copies from 'to' and to 'from'.
> 
> Looking at the assembler implementation it says the following
> 
>         # Support for void Copy::conjoint_words(void* from,
>          #                                       void* to,
>          #                                       size_t count)
> _Copy_conjoint_words:
>          stmdb    sp!, {r3 - r9, ip}
> 
> IE. It implies that it copies from 'from' and to 'to' in the comment, or in other words copies from memory pointed to by 'R0' to memory pointed to by 'R1' but in fact the implementation does the copy the other way around!
> 
> The quick and dirty fix would be to apply a (const *) cast to the 'to' arguments for ARM 32. However, I think this is just too broken and misleading.
> 
> My proposal is to fix this properly and have the assembler copy the correct way, and then delete the nasty conditionalisation on the calls. I also propose using symbolic names 'from' and 'to' rather than 'r0' and 'r1'.

Okay the conversion of r1 -> from, and r0 -> to, seems accurate.

I'm assuming the only actual bug was in the comment.

Thanks,
David

> Many thanks,
> Ed.
>