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

[Coleen Phillimore]

On Wed, 12 Jan 2022 18:47:29 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Much better solution from Kim - templates!
>
> 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.

Because the macosx compiler complains of ambiguity for the mov also if this is the only mov.

> 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.

This was here to match one of the Address constructors that I had.  With the template, it's no longer needed.

> 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.

This comment refers to Java code which more appropriately uses jlong.  Actually, the Java code uses long.  I'll revert this change.

> 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.

Ok. That'll give me less 'long' matches.

> 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?

This is a bug that Andrew pointed out. I could file a different CR for this and take it out.  It seemed a minor thing to include with this change though.

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

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