From: "Andreas Färber" <andreas.faerber@web.de>
To: Paul Brook <paul@codesourcery.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 0/4] tcg: Add debug facilities for TCGv
Date: Tue, 13 Dec 2011 13:43:04 +0100 [thread overview]
Message-ID: <4EE74858.40107@web.de> (raw)
In-Reply-To: <201112121558.44919.paul@codesourcery.com>
Am 12.12.2011 16:58, schrieb 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
[snip]
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".
> 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
[snip]
Again, that's where we disagree:
The whole point of TCGv and tl is to have variable-sized operations
scaling with target_long.
Thus, using them for fixed-size i32 or i64 operations is a semantic
error by definition. Whether or not an i64 target exists.
And I certainly don't want to knowingly introduce semantic errors in my
new code just so that at another time someone else can use that to
review a 64-bit port. That's just plain stupid. As the developer I must
know what semantics I am implementing for my target.
Note that the three choices are independent of this series, same holds
without. The difference is, my series offers a way to *flag* cases where
this has been ignored.
>> 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.
My stated requirement is that I want to detect ALL uses of TCGv_i32 with
tl functions and all uses of TCGv with i32 functions, be it an error or
a warning. Whether or not such consistency seems useful to you.
I have already given four examples to Peter, that you quoted previously.
Consider a uint32_t 8-bit status register on a 20-bit architecture - it
never scales to i64 so I know that TCGv/tl is definitely wrong!
Either point out something that's technically wrong with these patches
and I'll gladly fix it, or, again, propose a constructive solution.
Reappearing after a year and destructively objecting to patches is
something we've been through before.
Thanks,
Andreas
next prev parent reply other threads:[~2011-12-13 12:44 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-10 9:02 [Qemu-devel] [PATCH 0/4] tcg: Add debug facilities for TCGv Andreas Färber
2011-12-10 9:02 ` [Qemu-devel] [PATCH 1/4] tcg: Introduce {MAKE,GET}_TCGV_TL macros Andreas Färber
2011-12-10 9:02 ` [Qemu-devel] [PATCH 2/4] tcg: Convert *_tl*() macros to inline functions Andreas Färber
2011-12-10 21:06 ` [Qemu-devel] [PATCH v2] " Andreas Färber
2011-12-10 9:02 ` [Qemu-devel] [PATCH 3/4] tcg: Update TCGV_{UNUSED,EQUAL}() macros Andreas Färber
2011-12-10 9:02 ` [Qemu-devel] [PATCH 4/4] tcg: Allow to detect TCGv misuses Andreas Färber
2011-12-10 10:07 ` [Qemu-devel] [PATCH 0/4] tcg: Add debug facilities for TCGv Peter Maydell
2011-12-10 11:14 ` Andreas Färber
2011-12-11 23:28 ` Paul Brook
2011-12-12 10:39 ` Andreas Färber
2011-12-12 15:58 ` Paul Brook
2011-12-13 12:43 ` Andreas Färber [this message]
2011-12-13 16:26 ` Paul Brook
2011-12-14 11:41 ` Andreas Färber
2011-12-13 13:11 ` Andreas Färber
2011-12-13 16:23 ` Paul Brook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4EE74858.40107@web.de \
--to=andreas.faerber@web.de \
--cc=paul@codesourcery.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.