qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Chen Gang <chengang@emindsoft.com.cn>
To: Richard Henderson <rth@twiddle.net>,
	Peter Maydell <peter.maydell@linaro.org>,
	Chris Metcalf <cmetcalf@ezchip.com>
Cc: chenwei@emindsoft.com.cn, qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH v3 3/4] target-tilegx: Add double floating point implementation
Date: Sat, 12 Dec 2015 10:45:40 +0800	[thread overview]
Message-ID: <566B8A54.5090604@emindsoft.com.cn> (raw)
In-Reply-To: <566B6D44.1070303@twiddle.net>


On 12/12/15 08:41, Richard Henderson wrote:
> On 12/11/2015 03:38 PM, Chen Gang wrote:
>>
>> On 12/11/15 05:17, Richard Henderson wrote:
>>> On 12/10/2015 06:15 AM, Chen Gang wrote:
>>>> +#define TILEGX_F_MAN_HBIT   (1ULL << 59)
>>> ...
>>>> +static uint64_t fr_to_man(float64 d)
>>>> +{
>>>> +    uint64_t val = get_f64_man(d) << 7;
>>>> +
>>>> +    if (get_f64_exp(d)) {
>>>> +        val |= TILEGX_F_MAN_HBIT;
>>>> +    }
>>>> +
>>>> +    return val;
>>>> +}
>>>
>>> One presumes that "HBIT" is the ieee implicit one bit.
>>> A better name or better comments would help there.
>>>
>>
>> OK, thanks. And after think of again, I guess, the real hardware does
>> not use HBIT internally (use the full 64 bits as mantissa without HBIT).
> 
> It must do.  Otherwise the arithmetic doesn't work out.
> 

Oh, yes, and we have to use my original implementation (60 for mantissa,
4 bits for other using).

>> But what I have done is still OK (use 59 bits + 1 HBIT as mantissa), for
>> 59 bits are enough for double mantissa (52 bits). It makes the overflow
>> processing easier, but has to process mul operation specially.
> 
> What you have works.  But the mul operation isn't as special as you make it out -- aside from requiring at least 104 bits as intermediate -- in that when one implements what the hardware does, subtraction also may require significant normalization.
> 

I guess, you misunderstood what I said (my English is not quite well).

For mul, at least, it needs (104 - 1) bits, At present, we have 120 bits
for it (in fact, our mul generates 119 bits result). So it is enough.


>> According to floatsidf, it seems "4", but after I expanded the bits, I
>> guess, it is "7".
>>
>> /*
>>   * Double exp analyzing: (0x21b00 << 1) - 0x37(55) = 0x3ff
>>   *
>>   *   17  16  15  14  13  12  11  10   9   8   7    6   5   4   3   2   1   0
>>   *
>>   *    1   0   0   0   0   1   1   0   1   1   0    0   0   0   0   0   0   0
>>   *
>>   *    0   0   0   0   0   1   1   0   1   1   1    => 0x37(55)
>>   *
>>   *    0   1   1   1   1   1   1   1   1   1   1    => 0x3ff
>>   *
>>   */
> 
> That's the exponent within the flags temporary.  It has nothing to do with the position of the extracted mantissa.
> 

0x37(55) + 4 (guard bits) + 1 (HBIT) = 60 bits.

So, if the above is correct, the mantissa is 60 bits (with HBIT), and
bit 18 in flags for overflow, bit 19 for underflow (bit 20 must be for
sign).

> FWIW, the minimum shift would be 3, in order to properly implement rounding; if the hardware uses a shift of 4, that's fine too.
> 

I guess, so it uses 4 guard bits.

> What I would love to know is if the shift present in floatsidf is not really required; equally valid to adjust 0x21b00 by 4.  Meaning normalization would do a proper job with the entire given mantissa.  This would require better documentation, or access to hardware to verify.
> 

I guess, before call any fdouble insns, we can use the low 4 bits as
mantissa (e.g. calc mul), but when call any fdouble insn, we can not use
the lower 4 guard bits, so floatsidf has to shift 4 bits left.

>>>> +uint64_t helper_fdouble_addsub(CPUTLGState
>> And for my current implementation (I guess, it should be correct):
>>
>> typedef union TileGXFPDFmtV {
>>      struct {
>>          uint64_t mantissa : 60;   /* mantissa */
>>          uint64_t overflow : 1;    /* carry/overflow bit for absolute add/mul */
>>          uint64_t unknown1 : 3;    /* unknown */
> 
> I personally like to call all 4 of the top bits overflow.  But I have no idea what the real hardware actually does.
> 
>> In helper_fdouble_addsub(), both dest and srca are unpacked, so they are
>> within 60 bits. So one time absolute add are within 61 bits, so let bit
>> 61 as overflow bit is enough.
> 
> True.  But if all 4 top bits are considered overflow, then one could implement floatdidf fairly easily.  But I suspect that real hw doesn't work that way, or it would have already been done.
> 

So, I only assumed bit 60 is for overflow, the high 3 bits are unknown.

For me, if one bit for overflow is enough, the hardware will save the
other bits for another using (or are reserved for future).


Thanks.
-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

  reply	other threads:[~2015-12-12  2:43 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <56698865.8050901@emindsoft.com.cn>
2015-12-10 14:13 ` [Qemu-devel] [PATCH v3 1/4] target-tilegx: Add floating point shared functions Chen Gang
2015-12-10 14:15 ` [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation Chen Gang
2015-12-10 17:15   ` Richard Henderson
2015-12-10 20:18     ` Richard Henderson
2015-12-10 22:15       ` Chen Gang
2015-12-22 22:29         ` Chen Gang
2015-12-10 22:14     ` Chen Gang
2015-12-20 15:30       ` Chen Gang
2015-12-21 15:01         ` Richard Henderson
2015-12-21 18:54           ` Chen Gang
2015-12-22  2:00             ` Richard Henderson
2015-12-10 14:15 ` [Qemu-devel] [PATCH v3 3/4] target-tilegx: Add double " Chen Gang
2015-12-10 21:17   ` Richard Henderson
2015-12-11 23:38     ` Chen Gang
2015-12-12  0:41       ` Richard Henderson
2015-12-12  2:45         ` Chen Gang [this message]
2015-12-10 14:16 ` [Qemu-devel] [PATCH v3 4/4] target-tilegx: Integrate floating pointer implementation Chen Gang
2015-12-10 21:37   ` Richard Henderson
2015-12-10 21:44     ` Chen Gang
2015-12-10 14:26 ` [Qemu-devel] [PATCH v3 0/4] target-tilegx: Implement floating point instructions Chen Gang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=566B8A54.5090604@emindsoft.com.cn \
    --to=chengang@emindsoft.com.cn \
    --cc=chenwei@emindsoft.com.cn \
    --cc=cmetcalf@ezchip.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).