[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH v6 8/9] target-mips: Add nan2008 flavor of <CEIL|C

From: Leon Alrae
Subject: Re: [Qemu-arm] [PATCH v6 8/9] target-mips: Add nan2008 flavor of <CEIL|CVT|FLOOR|ROUND|TRUNC>.<L|W>.<S|D>
Date: Tue, 14 Jun 2016 15:17:53 +0100
User-agent: Mutt/1.5.21 (2010-09-15)

On Fri, Jun 10, 2016 at 09:12:12PM +0100, Maciej W. Rozycki wrote:
> On Fri, 10 Jun 2016, Aleksandar Markovic wrote:
> > The changes that make QEMU behavior the same as hardware behavior (in 
> > relation to CEIL, CVT, FLOOR, ROUND, TRUNC Mips instructions) are 
> > already contained in this patch.
>  Good, however that means that you've really combined two logically 
> separate changes into a single patch:
> 1. A bug fix for SoftFloat legacy-NaN (original) MIPS support, which has 
>    been there probably since forever (i.e. since the MIPS target was added 
>    to QEMU).

I've just done another round of review and as far as I can tell these
patches don't modify the legacy-NaN MIPS behaviour. I believe Aleksandar
was referring to new functionality (i.e. 2008 NaN) only.


> 2. A new feature for 2008-NaN MIPS support.
> To me it really looks like the two need to be separate patches, with the 
> bug fix applied first (or among any other bug fixes at the beginning) in 
> the patch set, or even as a separate change marked as a prerequisite for 
> the rest of the changes.
>  The bug fix will then be self-contained and more prominently exposed, 
> rather than being buried among feature additions.  It can then be 
> independently reviewed and likely more easily accepted as long as it is 
> technically correct.  It can also be cherry-picked and backported easily 
> if necessary, perhaps outside the upstream tree.
>  Review of the new feature set can then follow, once the bug(s) have been 
> fixed.
> > I just mentioned Mips-A / Mips-B / SoftFloat differences as an 
> > explanation/observation related to the change in this patch.
>  Maybe it's just myself, but from your description I got the impression 
> that your change preserves the status quo and the explanation merely 
> serves the purpose of documenting it.  Please consider rewriting it such 
> that it is unambiguous that the SoftFloat bug is being fixed with your 
> change.
>  Obviously once you've made the bug fix a separate change, it'll become 
> unambiguous naturally, as then you won't have the 2008-NaN feature along 
> it obfuscating the picture.
>   Maciej

reply via email to

[Prev in Thread] Current Thread [Next in Thread]