RFR: 8248404: AArch64: Remove uses of long and unsigned long [v8]

Kim Barrett kbarrett at openjdk.java.net
Wed Jan 12 19:02:34 UTC 2022


On Wed, 12 Jan 2022 15:14:10 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Tested with mach5 on linux-aarch64 and macosx-aarch64 on tier1-3 and below GHA for windows-aarch64 (once I open this PR).
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Much better solution from Kim - templates!

Changes requested by kbarrett (Reviewer).

src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp line 498:

> 496: 
> 497:   template<typename T, ENABLE_IF(std::is_integral<T>::value)>
> 498:   inline void mov(Register dst, T o)                      { mov_immediate64(dst, (uint64_t)o); }

I'm not sure why this isn't just
`inline void mov(Register dst, uint64_t o) { mov_immediate64(dst, o); }`
Unlike the `Address` case, I don't see any ambiguous implicit conversions here.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 1174:

> 1172:         assert(r->is_valid(), "bad oop arg");
> 1173:         if (r->is_stack()) {
> 1174:           __ ldr(temp_reg, Address(sp, (uint64_t)r->reg2stack() * VMRegImpl::stack_slot_size));

Why is the cast being added here?  Is it because the multiply can overflow an int, and this is really fix for a bug that is distinct from the cleanup in this PR?  There are a couple more like this later in this file.

src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp line 972:

> 970:  * Method entry for static native methods:
> 971:  *   int java.util.zip.CRC32.updateBytes(int crc, byte[] b, int off, int len)
> 972:  *   int java.util.zip.CRC32.updateByteBuffer(int crc, jlong buf, int off, int len)

Why not `int64_t` instead of `jlong`?   Similarly in a couple later places.

src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp line 997:

> 995:     // Calculate address of start element
> 996:     if (kind == Interpreter::java_util_zip_CRC32_updateByteBuffer) {
> 997:       __ ldr(buf, Address(esp, 2*wordSize)); // jlong buf

Maybe just drop the type in the comment?  Similarly in a couple later places.

src/hotspot/cpu/aarch64/universalUpcallHandler_aarch64.cpp line 74:

> 72:   // Capture prev stack pointer (stack arguments base)
> 73:   __ add(rscratch1, rfp, 16);   // Skip saved FP and LR
> 74:   Address slot = __ legitimize_address(Address(sp, layout.stack_args), wordSize, rscratch2);

Introduction of legitimize_address here seems unrelated to this PR.  What's this about?

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

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


More information about the hotspot-dev mailing list