Re: [Qemu-devel] [PATCH 0/4] tcg: Add debug facilities for TCGv

[Paul Brook]

> > Trying to make a 32-bit target "64-bit safe" without actually
> > implementing the 64-bit target is a complete waste of time.
> 
> That's where we disagree. I rather do things right from the start than
> leaving the cleanup work to someone else later on.
>
> > You've almost no chance of getting
> > it right.  In some cases the correct answer will be to use 32-bit
> > arithmetic, then sign/zero extend the result. In other cases the correct
> > answer will be to perform word size arithmetic.  Blindly picking one
> > just makes the bugs harder to find later.
> 
> This series picks nothing blindly. 

Sure it does - or at least it will when you get to ARM and m68k[1].  The whole 
point of your patch is to force developers to pick between an address sized 
value and a 32-bit value.  On a 32-bit target these are the same thing.  Not 
only are they the same thing, but most of the time the architecture doesn't 
even have the concept that they may be different.

Only when you introduce a variant with 64-bit addresses does the distinction 
become meaningful.  At that point they're actually different, and are 
trivially detected by the existing checks.

Ther are three ways to resolve a mismatch:
- Change everything to TCGv_i32.
- Change everything to TCGv.
- Add an explicit extension/truncation (compiles to no-op on 32-bit targets).

There's no way of the developer of a 32-bit architecture to know which of 
these a [hypothetical future] 64-bit architecture will require.  The fact they 
you've been forced to pick one (rather then leave the mismatch for the 64-bit 
porter to resolve) actually makes bugs harder to find.

> For me the most annoying issue was that tcg_gen_qemu_{ld,st}* needs TCGv.

You mean the value transferred is always TCGv sized, so ld32u requires an 
additional truncation before doing 32-bit arithmetic?  Fixing that is 
completely independent of making TCGv a separate type.

The whole point of TCGv is that it's an address sized value, so is the only 
real option for the address.  Constructing an address for a different sized 
value requires a target specific sign/szero extension or truncation/overflow 
check.

> > If you're trying to add support for targets where the primary word size
> > is neither 32 nor 64 then that's a completely different problem, and
> > probably not one that's worth solving.  In practice your port is going
> > to end up using 64- bit arithmetic and explicitly compensating for the
> > excess precision where necessary.
> 
> That's a different issue and not being addressed here.

Good.
 
> My point was that I have an inheritance hierarchy where (as opposed to,
> e.g., ppc and ppc64) both end up as TARGET_LONG_BITS==32, so I do not
> get a check for free by compiling both, and I do not want to introduce
> an artificial TARGET_LONG_BITS==64 architecture just to check that my
> tl==i32 target code is free of typos.

Huh? Now I'm completely confused.  If all your targets have T_L_B==32 how can 
mixing TCGv and TCGV_i32 be wrong? How do you know which of the 3 alternatives 
above is the right answer?

> Same issue if you pick only --target-list=<some>-softmmu BTW.

If you're only building a small subset of targets then clearly you won't see 
errors in the targets you omitted.  The only sane answer is to make sure you 
build (and preferably test) all the targets you have effected.

This is the same reason I object to code that is disabled by default.  If it 
isn't at least being built by the majority of developers (or at least any 
working in related areas) then it's going to bitrot rapidly, and probably 
won't work when someone does decide to turn it on.

> Just like with softfloat [u]int16 etc. types it's just too easy to
> forget _t somewhere (here: _i32) and to end up with a type you don't want.

I don't see how that is relevant.  TCGv (aka target_ulong) is a synonym for 
either TCGv_i32 (uint32_t) or TCGv_i64 (uint64_t).  If this is not true then 
we're completely screwed.

> If you have a better proposal how to introduce the checks I want, please
> let us hear it.

I still don't understand how your additional restructions are ever useful.
Please give an example of an actual error your checks will catch.

Paul

[1] m68k may never have a 64-bit variant, but ARM is a good example of a qemu 
target that is currently 32-bit but will grow a 64-bit variant in the not too 
distant future.