Re: [Qemu-devel] [PATCH v2 7/9] tcg-ppc64: Define TCG_TARGET_INSN_UNIT_SIZE

[Richard Henderson <rth@twiddle.net>]

On 04/01/2014 02:12 PM, Peter Maydell wrote:
>> -static uint16_t reloc_pc14_val(void *pc, tcg_target_long target)
>> +static uint16_t reloc_pc14_val(void *pc, tcg_insn_unit *target)
> 
> Should be tcg_insn_unit *pc like the others I guess?

Yep.

>> -    value += addend;
>> +    /* Note that we always use 0 for addend in calls to tcg_out_reloc.  */
> 
> Is this comment saying "assert(addend == 0)" ?
> 
>> +    tcg_insn_unit *target = (tcg_insn_unit *)value;

Err.. sure.

I sort of had in mind to eventually change the type of "value" to
tcg_insn_unit* and eliminate "addend".  But that's an interface change that has
to happen all at once, and that'll be easier after this set of changes is
complete across all targets.

>> -static const void * const qemu_st_helpers[16] = {
>> +static void * const qemu_st_helpers[16] = {
> 
> Why do we lose the extra 'const' here?

We'd have to carry the const all the way through all of the relocation and
pointer differencing functions.  It didn't seem worth it.

>>  #ifndef __APPLE__
>>      /* First emit adhoc function descriptor */
>>      tcg_out64(s, (uint64_t)s->code_ptr + 24); /* entry point */
>> -    s->code_ptr += 16;          /* skip TOC and environment pointer */
>> +    tcg_out64(s, 0);                          /* toc */
>> +    tcg_out64(s, 0);                          /* environment pointer */
> 
> This is a behaviour change, right? Is it a bugfix or just a "doesn't
> matter if we write zeros here or not" thing?

The later.  It seemed better to just write zeros than do "+= 2".


r~