RFR: 8170106: AArch64: Multiple JVMCI issues

[Andrew Haley]

On 21/11/16 16:29, Roland Schatz wrote:
> Hi,
> 
>> @@ -110,10 +110,11 @@
>>        * @param encoding the target machine encoding for the register
>>        * @param name the mnemonic name for the register
>>        * @param registerCategory the register category
>>        */
>>       public Register(int number, int encoding, String name, RegisterCategory registerCategory) {
>> + assert number < 64;
>>           this.number = number;
>>           this.name = name;
>>           this.registerCategory = registerCategory;
>>           this.encoding = encoding;
>>       }
> Looks like debug leftover. This is in plattform independent code, I 
> don't think we should restrict the number of registers allowed for a 
> single platform.
> 
> 
>>   void CodeInstaller::pd_patch_OopConstant(int pc_offset, Handle constant, TRAPS) {
>>     address pc = _instructions->start() + pc_offset;
>>     Handle obj = HotSpotObjectConstantImpl::object(constant);
>>     jobject value = JNIHandles::make_local(obj());
>> - if (HotSpotObjectConstantImpl::compressed(constant)) {
>> - int oop_index = _oop_recorder->find_index(value);
>> - RelocationHolder rspec = oop_Relocation::spec(oop_index);
>> - _instructions->relocate(pc, rspec, 1);
>> - Unimplemented();
>> - } else {
>> - NativeMovConstReg* move = nativeMovConstReg_at(pc);
>> - move->set_data((intptr_t) value);
>> + MacroAssembler::patch_oop(pc, (address)obj());
>>       int oop_index = _oop_recorder->find_index(value);
>>       RelocationHolder rspec = oop_Relocation::spec(oop_index);
>>       _instructions->relocate(pc, rspec);
>> - }
>>   }
> This is now ignoring the `HotSpotObjectConstantImpl::compressed` field.
> 
> Looks like the `MacroAssembler::patch_oop` function does the correct 
> thing based on what instruction `pc` points to. I think we should put an 
> assertion in here that checks whether the flag is correct. I guess in a 
> product build it's ok to swallow this kind of error, but in a fastdebug 
> build I think we should fail if the compiler produces mismatching 
> instructions and data patches. It makes debugging the compiler a lot 
> easier...

OK, done.

http://cr.openjdk.java.net/~aph/8170106-2

I added one more hunk:

diff --git a/src/cpu/aarch64/vm/assembler_aarch64.hpp b/src/cpu/aarch64/vm/assembler_aarch64.hpp
--- a/src/cpu/aarch64/vm/assembler_aarch64.hpp
+++ b/src/cpu/aarch64/vm/assembler_aarch64.hpp
@@ -848,7 +848,7 @@
   // architecture.  In debug mode we shrink it in order to test
   // trampolines, but not so small that branches in the interpreter
   // are out of range.
-  static const unsigned long branch_range = NOT_DEBUG(128 * M) DEBUG_ONLY(2 * M);
+  static const unsigned long branch_range = INCLUDE_JVMCI ? 128 * M : NOT_DEBUG(128 * M) DEBUG_ONLY(2 * M);

because there isn't support for trampolines in Graal yet.  That will be
fixed, but not today.

Andrew.