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

[Paul Brook <paul@codesourcery.com>]

> >>> 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
> 
> No, start by reading the git summary. These four patches don't touch
> target-* at all.
>
> This is intentionally NOT some Coccinelle script running wild doing
> refactorings. That's what I would call "blindly".

IIUC these four patches do precicely nothing. They simply add a pile of dead 
code. If you don't intent on making these checks pass for all targets, and 
enabling them for at least debug builds then I strongly object.  These checks 
will bitrot rapidly if not routinely enforced.

> > 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
> 
> Again, that's where we disagree:
> 
> The whole point of TCGv and tl is to have variable-sized operations
> scaling with target_long.

So you're claiming that a 32-bit only target can somehow distinguish between a 
32-bit value, and a value that is architecturally defined to always be 32-bit.  
I don't believe any useful determination can be made in this case.

For targets with different target_ulong variants simply building those 
variants with the existing checks will already highlight any mismatches.

AFAICS the best you can do is say that 32-bit only targets should never use 
TCGv. While that might be a nice idea in theory, the opposite is true for the 
current code base.  This is because the original TCG implementation did not do 
any static typing, i.e. everything was TCGv.  It was only later that I gave 
TCG variables a static size.  I see no practical harm from leaving that as-is.  
Introducing a substantial amount of churn and extra complication to achieve a 
purely theoretical goal is a bad idea.

> I have already given four examples to Peter, that you quoted previously.

The examples I quoted where where TCGv and TCGv_i32 were mixed, but I don't 
see how any of those could possibly cause incorrect behovior.  If the two were 
ever different then this would already fail to compile.

So I'll ask again: Please give a worked example of a programming error that is 
caught by your new restrictions. Feel free to use hypothetical code and/or 
targets.

Paul