[aarch64-port-dev ] Call ICache::invalidate_range()

[Andrew Haley]

On 11/06/2014 09:37 AM, Edward Nevill wrote:
> Hi Andrew,
> 
> Thanks for this. A couple of comments inline.
> 
> On Wed, 2014-11-05 at 14:05 +0000, Andrew Haley wrote:
>>  	guarantee(((dest >> size) << size) == dest, "misaligned target");
>> +	size = 2;
> 
> The context here is
> 
>       if (Instruction_aarch64::extract(insn2, 29, 24) == 0b111001 &&
>                 Instruction_aarch64::extract(insn, 4, 0) ==
>                         Instruction_aarch64::extract(insn2, 9, 5)) {
>         // Load/store register (unsigned immediate)
> // ------------- VVVV LOOK HERE
>         unsigned size = Instruction_aarch64::extract(insn2, 31, 30);
> // ------------- ^^^^ LOOK HERE
>         Instruction_aarch64::patch(branch + sizeof (unsigned),
>                                     21, 10, offset_lo >> size);
>         guarantee(((dest >> size) << size) == dest, "misaligned target");
>         size = 2;
> 
> The local definition of size is shadowing your function level definition.

Ah, okay.  I'll think of a better name.

> (This is why I hate C and all C based languages).
> 
>>        } else{
>>  	MacroAssembler::patch_oop(addr(), x);
>> +	size = NativeMovConstReg::instruction_size;
>>        }
> 
> patch_oop can patch 2 instructions, not just 1?

Sorry, that's too obscure for me.  What are you asking?

> void MacroAssembler::patch_oop(address insn_addr, address o) {
>   unsigned insn = *(unsigned*)insn_addr;
>   if (Instruction_aarch64::extract(insn, 31, 21) == 0b11010010101) {
>       // Move narrow constant
>       assert(nativeInstruction_at(insn_addr+4)->is_movk(), "wrong insns in patch");
>       narrowOop n = oopDesc::encode_heap_oop((oop)o);
>       Instruction_aarch64::patch(insn_addr, 20, 5, n >> 16);      // <<< LOOK HERE
>       Instruction_aarch64::patch(insn_addr+4, 20, 5, n & 0xffff); // <<< LOOK HERE
>   } else {
>     pd_patch_instruction(insn_addr, o);
>   }
> }

I'll grant that still looks wrong: there can be 2 or 3 instructions
there, depending on narrow OOPs.  I don't think it can ever matter,
but I suppose it should be fixed.  I'll think of something better.

Andrew.