|
From: | Andreas Färber |
Subject: | Re: [Qemu-devel] [PATCH 1/6] softfloat: remove HPPA specific code |
Date: | Wed, 5 Jan 2011 09:15:43 +0100 |
Am 05.01.2011 um 00:56 schrieb Aurelien Jarno:
On Tue, Jan 04, 2011 at 11:53:01PM +0100, Andreas Färber wrote:Am 04.01.2011 um 21:07 schrieb Aurelien Jarno:On Tue, Jan 04, 2011 at 08:54:04PM +0100, Andreas Färber wrote:Am 03.01.2011 um 15:34 schrieb Aurelien Jarno:We don't have any HPPA target, so let's remove HPPA specific code. It can be re-added when someone adds an HPPA target. Signed-off-by: Aurelien Jarno <address@hidden>There actually is such a project on SourceForge [1, 2].The project hasn't seen any commit for 1.5 year. It looks like dead.As we have begun to collect in the forks thread, there's many multi- month-old repos around with features not in upstream. Even "dead"doesn't mean useless. Considering that linux-user is incomplete even on amd64, I don't see why we shouldn't have target-hppa in master. Then it would at least allow for compile-testing and would benefit from general refactoring rather than bitrotting. All it takes is someone to step upfor upstreaming patches, and I do not feel qualified to volunteer for that architecture.You are comparing apples and oranges, forks and dead code. linux- user on x86_64 is able to run binaries. HPPA code has still to be ported to TCG.Yes it still uses dyngen.
Oh. I was pretty sure I saw TCG host support patches...
Does it really hurt to leave TARGET_HPPA around?It means writing code for this target, in the current case for floatXX_maybe_silence_NaN(). I don't see the point of writing and maintaining unused code if we don't get the insurance the target is going to be merged later. Unless someone volunteer to maintain this code.For new code, #elif defined(TARGET_HPPA) #error Target not supported yet. ...shouldn't be too much work if you already handle architecture specificsThat makes work, code less readable, without any benefit. The code shouldbe added when we have a real fork to reintegrate back. Should we already start adding code provision about TARGET_DPTR (themarvelous CPU I have dreamed last night)? And about TARGET_IPU (the oneI have dreamed the night before)?and is different from ripping out existing code, as you seemed to suggestfor linux-user.Strange interpretation for "I personnally don't have a lot of interestin linux-user, so I will let the linux-user maintainer (Cc) to decide."
Really? I think you're completely missing my point. Quote: "softfloat: remove HPPA specific codeWe don't have any HPPA target, so let's remove HPPA specific code. [...]"
Quote: "> Do we want to get rid of the one remaining TARGET_HPPA which is in linux-user/syscall_defs.h?
[...] I will let the linux-user maintainer (Cc) [...] decide."I'm saying that the justification of your patch and action towards Riku is wrong. Patch 1/6 should be dropped, don't remove code just because "we don't have any HPPA target". Because obviously if anyone rebases hppa.git to such a master HEAD, the code will be gone just as well.
What you've been interpreting into this is that you should go through hoops and newly add some TARGET_FOOBAR crap, which is not what I said. If you add some silence_magic_nan() and don't *know* how the code for HPPA should look like based on the pre-existent code, then I suggested to simply stub it out for such platforms (whether based on #elif defined(TARGET_foo) or a mere #else is pretty irrelevant to me). So in the end we will end up with patches that don't support hppa out of the box but which don't needlessly remove SNAN_BIT_IS_ONE and float{32,64}_default_nan(). I fail to see how these three macros hurt without digging deeper into your patches. If they do, then your patch 1/6 is lacking a proper description!
Point is, don't remove valuable code just because it's not being used in upstream *yet*.
In that context, there would've been no need to bring the linux-user maintainer into this who could've complained earlier if HPPA caused him any trouble. I never stumbled across any prominent mention of hppa in his patch series.
Hope that explains, Andreas
[Prev in Thread] | Current Thread | [Next in Thread] |