RFR: aarch32: ARM 32 build broken after 8165929 (original) (raw)

David Holmes david.holmes at oracle.com
Fri Mar 16 02:28:01 UTC 2018

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/oscpu/linuxarm/copylinuxarm.inline.hpp:33:54: error: invalid conversion from 'const HeapWord*' to 'HeapWord*' [-fpermissive] Copyconjointwords(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 Copyconjointwords(HeapWord* from, HeapWord* to, sizet count); - void Copydisjointwords(HeapWord* from, HeapWord* to, sizet count); + void Copyconjointwords(const HeapWord* from, HeapWord* to, sizet count); + void Copydisjointwords(const HeapWord* from, HeapWord* to, sizet count); The problem in the ARM 32 port occurs in code like the following static void pdconjointwords(const HeapWord* from, HeapWord* to, sizet count) { #ifdef AARCH64 Copyconjointwords(from, to, count * HeapWordSize); #else // NOTE: Copy* functions on 32-bit ARM expect "to" and "from" arguments in reversed order Copyconjointwords(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::conjointwords(void* from, # void* to, # sizet count) Copyconjointwords: 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.

More information about the hotspot-dev mailing list