[aarch64-port-dev ] RFR: 8139674: aarch64: guarantee failure in TestOptionsWithRanges.java

[Andrew Dinn]

On 16/10/15 10:02, Roland Westrelin wrote:
>> I don't think removing next line is right:
>>
>> -        add(mdp, mdp, to_add);
>>
>> it shifts to next metadata cell.
> 
> Yes, we shift to the next argument slot. We also do:
> 
> off_to_args += to_add;
> 
> and then data in the slot is addressed with:
> 
> in_bytes(TypeEntriesAtCall::stack_slot_offset(i))-off_to_args
> 
> that is relative to the current argument slot.
> 
> before the change. After the change, it’s adjusted to:
> 
> in_bytes(TypeEntriesAtCall::stack_slot_offset(i))
> 
> that is relative to the beginning of the space allocated for all arguments. So I think it’s good.

I agree with Roland that this change set is good. As he points out, in
the new code off_to_args is being updated every time round the loop

  1575         off_to_args += to_add;

So, the add instructions generated at

  1557           add(rscratch1, mdp, off_to_args);

and

  1583       add(rscratch1, mdp, off_to_args);

will be inserting different positive constants into the generated
instruction stream depending on the current value of off_to_args.

> This said, this:
> 
>>> What is happening is that the code in InterpreterMacroAssembler::profile_arguments_type adjusts mdp to point past the arguments and uses negative offsets to index fields at the start of the method data structure.
> 
> is not correct I think. We don’t move past the arguments. We move to the beginning of the slot dedicated to the next argument, not past it. The only source of negative offsets I can see is:
> 
> 1555           ldr(tmp, Address(mdp, in_bytes(TypeEntriesAtCall::cell_count_offset())-off_to_args));

What Ed said made sense to me. The old code executed

1550       add(mdp, mdp, off_to_args);

before the loop i.e. it left mdp pointing just past the args. Subsequent
ldr instructions then used a negative offset from this updated mdp
register to address the arg slots which precede the slot addressed by mdp.

Anyway, whatever the status regarding validity/clarity of the
explanation Ed provided in the accompanying mail note the actual code
changes look correct to me.

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in UK and Wales under Company Registration No. 3798903
Directors: Michael Cunningham (USA), Matt Parson (USA), Charlie Peters
(USA), Michael O'Neill (Ireland)