On Tue, May 14, 2013 at 2:01 PM, Claudio Fontana
<claudio.font...@huawei.com> wrote:
[...]
>>> +static void tcg_target_qemu_prologue(TCGContext *s)
>>> +{
>>> + int r;
>>> + int frame_size; /* number of 16 byte items */
>>> +
>>> + /* we need to save (FP, LR) and X19 to X28 */
>>> + frame_size = (1) + (TCG_REG_X27 - TCG_REG_X19) / 2 + 1;
>>
>> The comment says "X19 to X28" and the code does X27 - X19:
>> which is right?
>
> The comment is right, and the code is technically working but it shows as
> misleading for the reader.
> I will rewrite the callee-saved registers' contribution to the frame size (in
> 16 byte elements) as:
>
> (TCG_REG_X28 - TCG_REG_X19) / 2 + 1;
Shouldn't that be like this?
((TCG_REG_X28 - TCG_REG_X19 + 1) + 1) / 2 + 1;
The last +1 is for FP,LR as you explained.
The first +1 is needed to count the number of regs in the
interval [x19,x28].
The second +1 is needed because if the number of regs
is odd, you want to round up and not down.
Here that'd give us (9+1+1)/2+1 = 6.
Of course that's nitpicking because the callee-saved regs
shouldn't change :-)
>>
>> Why the brackets round the first '1' ?
>
> It represents the (FP, LR) pair, so the () should help the reader notice that
> it refers
> to the couple (FP, LR) mentioned in the comment just above.
> It clearly failed, so I will try to align the comment and the code better.
>
>>
>>> +
>>> + /* push (fp, lr) and update sp to final frame size */
>>> + tcg_out_push_p(s, TCG_REG_FP, TCG_REG_LR, frame_size);
>>> +
>>> + /* FP -> frame chain */
>>> + tcg_out_movr_sp(s, 1, TCG_REG_FP, TCG_REG_SP);
>>> +
>>> + /* store callee-preserved regs x19..x28 */
>>> + for (r = TCG_REG_X19; r <= TCG_REG_X27; r += 2) {
Shouldn't the comparison be against TCG_REG_X28? It'd
be more readable.
>>> + int idx; idx = (r - TCG_REG_X19) / 2 + 1;
>>> + tcg_out_store_p(s, r, r + 1, idx);
>>> + }
>>> +
>>> + tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
>>> + tcg_out_gotor(s, tcg_target_call_iarg_regs[1]);
>>> +
>>> + tb_ret_addr = s->code_ptr;
>>> +
>>> + /* restore registers x19..x28 */
>>> + for (r = TCG_REG_X19; r <= TCG_REG_X27; r += 2) {
Ditto.
Thanks,
Laurent
>>> + int idx; idx = (r - TCG_REG_X19) / 2 + 1;
>>> + tcg_out_load_p(s, r, r + 1, idx);
>>> + }
>>> +
>>> + /* pop (fp, lr), restore sp to previous frame, return */
>>> + tcg_out_pop_p(s, TCG_REG_FP, TCG_REG_LR, frame_size);
>>> + tcg_out_ret(s);
>>> +}
