Re: [Qemu-devel] [PATCH 03/14] target-ppc: use separate indices for various translation modes

[Richard Henderson <rth@twiddle.net>]

On 09/16/2014 11:49 AM, Peter Maydell wrote:
> On 16 September 2014 10:20, Tom Musta <tommusta@gmail.com> wrote:
>>
>>   1389      /* Compensate for very large offsets.  */
>>   1390      if (add_off >= 0x8000) {
>>   1391          /* Most target env are smaller than 32k; none are larger than 64k.
>>   1392             Simplify the logic here merely to offset by 0x7ff0, giving us a
>>   1393             range just shy of 64k.  Check this assumption.  */
>>   1394          QEMU_BUILD_BUG_ON(offsetof(CPUArchState,
>>   1395                                     tlb_table[NB_MMU_MODES - 1][1])
>>   1396                            > 0x7ff0 + 0x7fff);
>>   1397          tcg_out32(s, ADDI | TAI(TCG_REG_TMP1, base, 0x7ff0));
>>   1398          base = TCG_REG_TMP1;
>>   1399          cmp_off -= 0x7ff0;
>>   1400          add_off -= 0x7ff0;
>>   1401      }
> 
> Is it possible to promote this BUILD_BUG_ON from "only on
> PPC hosts" to "on all builds" ? It's really checking a
> property of the target CPU's code, not a property of
> the TCG backend, and I bet a lot of our backends don't
> get built very often so we could easily miss breakage.
> I guess you'd need to define and check a worst-case value
> in a common header somewhere.

Meh.  It is a property of the tcg backend, in that it is a property of the code
that immediately follows.  And that's what makes the BUG_ON clear and obvious, IMO.

For what it's worth, ppc as written has the smallest constraint of the current
backends, and I'm fairly confident that'll get built often-ish.

If you've got a rearrangement that puts the assert somewhere else, and keeps
the magic numbers understandable... I'll certainly have a look, but I don't see
how to retain the obviousness with a different placement.


r~