RFR: 8139674: aarch64: guarantee failure in TestOptionsWithRanges.java

Edward Nevill edward.nevill at gmail.com
Fri Oct 16 09:12:41 UTC 2015


Hi Vladimir,

Thanks for the review.

On Fri, 2015-10-16 at 08:53 +0800, Vladimir Kozlov wrote:
> I don't think removing next line is right:
> 
> -        add(mdp, mdp, to_add);
> 
> it shifts to next metadata cell.

I believe removal of this line is correct. The old code kept mdp pointing at the current metadata cell that it was processing. This is what lead to the large negative offsets when it was trying to index the two fields at the start of the metadata structure.

So in the old code, to process each metatdata cell it did.

    Address mdo_arg_addr(mdp, in_bytes(TypeEntriesAtCall::argument_type_offset(i))-off_to_args);
    profile_obj_type(tmp, mdo_arg_addr);

In the revised code it does

    Address mdo_arg_addr(mdp, in_bytes(TypeEntriesAtCall::argument_type_offset(i)));
    profile_obj_type(tmp, mdo_arg_addr);

'off_to_args' is updated by 'to_add' each time round the loop.

    off_to_args += to_add;

So in the old code the address of the metadata cell was calculated as

    mdp + in_bytes(TypeEntriesAtCall::argument_type_offset(i))-off_to_args

Note: 'in_bytes(TypeEntriesAtCall::argument_type_offset(i))-off_to_args' always evaluates to 0!

In the revised code the address is calculated as 'in_bytes(TypeEntriesAtCall::argument_type_offset(i))' which evaluates to the same address since mdp is no longer offset by 'off_to_args'.

> 
> Also both exit paths execute the same instruction:
> 
> +      add(rscratch1, mdp, off_to_args);
> 
> may be you can simplify exit without them:
> 
>         bind(done);
> +      add(mdp, mdp, off_to_args);

I don't believe the exit can be simplified.

The reason is that 'off_to_args' is different for each iteration around the loop. So in assembler what we will have is

    add   x8, x0, #8 // x0 == mdp, offset to arg 0 == 8
    blt   done
    ...
    add   x8, x0, #0x18 // arg 1 offset = 0x18
    blt   done
    ....

    add   x8, x0, #0x108 // fallthru case
done:
    mov   x0, x8

So, mdp is updated by a different immediate value in each case, so we cannot merge them into a single case

done:
    add   x0, x0, #N  // N == ???

Thanks for your help,
Ed.




More information about the hotspot-compiler-dev mailing list