[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: Maciej W. Rozycki
Subject: Re: [Qemu-arm] [PATCH v6 8/9] target-mips: Add nan2008 flavor of <CEIL|CVT|FLOOR|ROUND|TRUNC>.<L|W>.<S|D>
Date: Fri, 10 Jun 2016 21:12:12 +0100
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)

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).

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 

> 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 

 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.


reply via email to

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