RFR: aarch32: ARM 32 build broken after 8165929 (original) (raw)
Edward Nevill edward.nevill at gmail.com
Thu Mar 15 21:40:37 UTC 2018
- Previous message: RFR(S): 8199698: Change 8199275 breaks template instantiation for xlC (and potentially other compliers)
- Next message: RFR: aarch32: ARM 32 build broken after 8165929
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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'.
Many thanks, Ed.
- Previous message: RFR(S): 8199698: Change 8199275 breaks template instantiation for xlC (and potentially other compliers)
- Next message: RFR: aarch32: ARM 32 build broken after 8165929
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]