Re: [Qemu-devel] [PATCH 1/6] softfloat: remove HPPA specific code

[Andreas Färber]

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  
> > up
> > for 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  
> > specifics
>
> That makes work, code less readable, without any benefit. The code  
> should
> be added when we have a real fork to reintegrate back.
>
> Should we already start adding code provision about TARGET_DPTR (the
> marvelous CPU I have dreamed last night)? And about TARGET_IPU (the  
> one
> I have dreamed the night before)?
>
> > and is different from ripping out existing code, as you seemed to  
> > suggest
> > for linux-user.
>
> Strange interpretation for "I personnally don't have a lot of interest
> in 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 code

We 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