From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=34365 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Pq81C-0003QU-21
	for qemu-devel@nongnu.org; Thu, 17 Feb 2011 12:51:55 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <christophe.lyon@st.com>) id 1Pq81A-0001Ml-Qn
	for qemu-devel@nongnu.org; Thu, 17 Feb 2011 12:51:53 -0500
Received: from eu1sys200aog103.obsmtp.com ([207.126.144.115]:57591)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <christophe.lyon@st.com>) id 1Pq81A-0001MA-Jr
	for qemu-devel@nongnu.org; Thu, 17 Feb 2011 12:51:52 -0500
Message-ID: <4D5D6033.8000807@st.com>
Date: Thu, 17 Feb 2011 18:51:47 +0100
From: Christophe Lyon <christophe.lyon@st.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 2/3] target-arm: fix support for vrecpe.
References: <1297879186-12670-1-git-send-email-christophe.lyon@st.com>	<1297879186-12670-3-git-send-email-christophe.lyon@st.com>
	<AANLkTikUKMx2iWGhz-V8nCG++TrK2w0CBe_kONqzmCv_@mail.gmail.com>
In-Reply-To: <AANLkTikUKMx2iWGhz-V8nCG++TrK2w0CBe_kONqzmCv_@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>


>> +    float_status *s = &env->vfp.standard_fp_status;
>> +    float64 one = int64_to_float64(1, s);
> You don't need to create a variable for this, float64_one
> will do what you want.
OK

 
>> +    /* q = (int)(a * 512.0) */
>> +    float64 x512 = int64_to_float64(512, s);
>> +    float64 q = float64_mul(x512, a, s);
>> +    int64_t q_int = float64_to_int64_round_to_zero(q, s);
>> +
>> +    /* r = 1.0 / (((double)q + 0.5) / 512.0) */
>> +    q = int64_to_float64(q_int, s);
>> +    float64 half = float64_div(one, int64_to_float64(2, s), s);
> ...and a runtime division just to get a constant 0.5?
> Better to just make_float64() on the appropriate bit
> pattern, I think.

It makes sense.
Then, what about using the right bit patterns for 512 and 256?

Actually, for these last two, I mimicked recps and rsqrts which build constants 2 and 3. I could add another patch to address this point.
 

>> +    if (float32_is_any_nan(a)) {
>> +        return float32_default_nan;
> This won't set InvalidOp if the input is a signalling NaN.
Thanks for the notice, I have no means of testing those cases.

Christophe.