Re: [PATCH] linux-user: syscall: ioctls: support DRM_IOCTL_I915_GETPARAM

2020-07-18 Thread Chen Gang
On 2020/7/14 上午2:46, Laurent Vivier wrote:
>> +gparam->value = lock_user(VERIFY_WRITE, target_gparam->value,
>> + sizeof(*gparam->value), 0);
> 
> I don't think you should use directly the guest memory.
> You should have something like that:
> 
>  int value;
> 
>  gparam->value = 
> 
>> +if (!gparam->value) {
>> +unlock_user_struct(target_gparam, arg, 0);
>> +return -TARGET_EFAULT;
>> +}
>> +
>> +ret = get_errno(safe_ioctl(fd, ie->host_cmd, gparam));
> 
> and then:
> 
> put_user_s32(value, target_gparam->value);
> 

OK, thanks. It will be better.

>> +
>> +unlock_user(gparam->value, target_gparam->value, 
>> sizeof(*gparam->value));
>> +unlock_user_struct(target_gparam, arg, 0);
>> +return ret;
>> +}
>> +
>> +static abi_long do_ioctl_drm_i915(const IOCTLEntry *ie, uint8_t *buf_temp,
>> +  int fd, int cmd, abi_long arg)
>> +{
>> +switch (ie->host_cmd) {
>> +case DRM_IOCTL_I915_GETPARAM:
>> +return do_ioctl_drm_i915_getparam(ie,
>> +  (struct drm_i915_getparam 
>> *)buf_temp,
>> +  fd, arg);
>> +default:
>> +return -TARGET_ENOSYS;
>> +}
>> +}
> 
> There is a better way to register a struct with convertion functions:
> you might use STRUCT_SPECIAL() to declare the structure rather than
> IOCTL_SPECIAL() to declare the ioctl command.
> (I think STRUCT_SPECIAL() could also be used with DRM_IOCTL_VERSION)
> 

For me, STRUCT_SPECIAL sounds good for the complex structures, but for
drm_i915_getparam which is simple enough, it is not quite suitable.

For me, STRUCT_SPECIAL is much suitable for DRM_IOCTL_VERSION.

Welcome your additional ideas.

>>  
>>  static IOCTLEntry ioctl_entries[] = {
>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index 3c261cff0e..9082f6c2bc 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -1170,6 +1170,9 @@ struct target_rtc_pll_info {
>>  /* drm ioctls */
>>  #define TARGET_DRM_IOCTL_VERSION  TARGET_IOWRU('d', 0x00)
>>  
>> +/* drm i915 ioctls */
>> +#define TARGET_DRM_IOCTL_I915_GETPARAM  TARGET_IOWRU('d', 0x46)
> 
> why do you use the U variant?
> 
> TARGET_IOWR('d', 0x46, struct target_drm_i915_getparam)
> 

Because qemu will automatically set the size with the target structure
size in syscall_init(). It'll be more easier. e.g. usb ioctls and device
mapper ioctls also do like this.





Re: [PATCH v7] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-07-08 Thread Chen Gang
I shall try to send another DRM_IOCTL_* patches within this weekend.

Thanks.

On 2020/6/29 下午7:05, Laurent Vivier wrote:
> Le 05/06/2020 à 03:32, cheng...@emindsoft.com.cn a écrit :
>> From: Chen Gang 
>>
>> Another DRM_IOCTL_* commands will be done later.
>>
>> Signed-off-by: Chen Gang 
>>
...
> 
> Applied to my branch linux-user-for-5.1
> 
> Thanks,
> Laurent
> 






Re: [PATCH] [PATCH v6] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-06-04 Thread Chen Gang
That sounds good, I'll send patch v7, thanks.

On 2020/6/4 下午5:10, Laurent Vivier wrote:
> Le 04/06/2020 à 03:45, cheng...@emindsoft.com.cn a écrit :
>> From: Chen Gang 
>>
>> Another DRM_IOCTL_* commands will be done later.
>>
>> Signed-off-by: Chen Gang 
>> ---
>>  configure  | 10 
>>  linux-user/ioctls.h|  5 ++
>>  linux-user/syscall.c   | 95 ++
>>  linux-user/syscall_defs.h  | 15 ++
>>  linux-user/syscall_types.h | 11 +
>>  5 files changed, 136 insertions(+)
>>
>> diff --git a/configure b/configure
>> index f087d2bcd1..389dbb1d09 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3136,6 +3136,13 @@ if ! check_include "ifaddrs.h" ; then
>>have_ifaddrs_h=no
>>  fi
>>  
>> +#
>> +# libdrm check
>> +have_drm_h=no
>> +if check_include "libdrm/drm.h" ; then
>> +have_drm_h=yes
>> +fi
>> +
>>  ##
>>  # VTE probe
>>  
>> @@ -7127,6 +7134,9 @@ fi
>>  if test "$have_ifaddrs_h" = "yes" ; then
>>  echo "HAVE_IFADDRS_H=y" >> $config_host_mak
>>  fi
>> +if test "$have_drm_h" = "yes" ; then
>> +  echo "HAVE_DRM_H=y" >> $config_host_mak
>> +fi
>>  if test "$have_broken_size_max" = "yes" ; then
>>  echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
>>  fi
>> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
>> index 0defa1d8c1..f2e2fa9c87 100644
>> --- a/linux-user/ioctls.h
>> +++ b/linux-user/ioctls.h
>> @@ -574,6 +574,11 @@
>>IOCTL_SPECIAL(SIOCDELRT, IOC_W, do_ioctl_rt,
>>  MK_PTR(MK_STRUCT(STRUCT_rtentry)))
>>  
>> +#ifdef HAVE_DRM_H
>> +  IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm,
>> +MK_PTR(MK_STRUCT(STRUCT_drm_version)))
>> +#endif
>> +
>>  #ifdef TARGET_TIOCSTART
>>IOCTL_IGNORE(TIOCSTART)
>>IOCTL_IGNORE(TIOCSTOP)
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 7f6700c54e..1744e7acc7 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -112,6 +112,9 @@
>>  #include 
>>  #include 
>>  #include 
>> +#ifdef HAVE_DRM_H
>> +#include 
>> +#endif
>>  #include "linux_loop.h"
>>  #include "uname.h"
>>  
>> @@ -5276,6 +5279,98 @@ static abi_long do_ioctl_tiocgptpeer(const IOCTLEntry 
>> *ie, uint8_t *buf_temp,
>>  }
>>  #endif
>>  
>> +#ifdef HAVE_DRM_H
>> +
>> +static void unlock_drm_version(struct drm_version *host_ver)
>> +{
>> +unlock_user(host_ver->name, 0UL, 0);
>> +unlock_user(host_ver->date, 0UL, 0);
>> +unlock_user(host_ver->desc, 0UL, 0);
>> +}
> 
> This is correct but I would prefer to not have this kind of function
> with 0UL parameter.
> 
> You can use target_ver for the guest pointer and host_ver for the len,
> guest pointer will be ignored if host pointer is NULL.
> 
> You can have a generic function that not copies data in case of error:
> 
> static void unlock_drm_strings(struct drm_version *host_ver,
>struct target_drm_version *target_ver,
>int copy)
> {
> unlock_user(host_ver->name, target_ver->name,
> copy ? host_ver->name_len : 0);
> ...
> 
>> +
>> +static inline abi_long target_to_host_drmversion(struct drm_version 
>> *host_ver,
>> +  struct target_drm_version 
>> *target_ver)
>> +{
>> +memset(host_ver, 0, sizeof(*host_ver));
>> +
>> +__get_user(host_ver->name_len, _ver->name_len);
>> +if (host_ver->name_len) {
>> +host_ver->name = lock_user(VERIFY_WRITE, target_ver->name,
>> +   target_ver->name_len, 0);
>> +if (!host_ver->name) {
>> +goto err;
>> +}
>> +}
>> +
>> +__get_user(host_ver->date_len, _ver->date_len);
>> +if (host_ver->date_len) {
>> +host_ver->date = lock_user(VERIFY_WRITE, target_ver->date,
>> +   target_ver->date_len, 0);
>> +if (!host_ver->date) {
>> +goto err;
>> +}
>> +}
>> +
>> +__get_user(host_ver->

Re: [PATCH v5] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-06-03 Thread Chen Gang
OK, thanks. I'll send patch v6. :)

On 2020/6/3 下午8:03, Laurent Vivier wrote:
> Le 03/06/2020 à 13:05, Chen Gang a écrit :
>> On 2020/6/3 下午5:49, Laurent Vivier wrote:
>>> Le 03/06/2020 à 03:08, cheng...@emindsoft.com.cn a écrit :
>>>> +#ifdef HAVE_DRM_H
>>>> +
>>>> +static void unlock_drm_version(struct drm_version *host_ver)
>>>> +{
>>>> +if (host_ver->name) {
>>>> +unlock_user(host_ver->name, 0UL, 0);
>>>
>>> unlock_user() allows to have a NULL host pointer parameter, so you don't
>>> need to check. But you must provide the target pointer, with the length.
>>> The same below.
>>>
>>
>> As far as I know, the unlock_user is defined in
>> include/exec/softmmu-semi.h, which only checks the len before calling
>> cpu_memory_rw_debug, and only calls free() for the host pointer.
>>
>> So we have to be sure that the host pointer must be valid. When we pass
>> 0 length to unlock_user, we want it to free host pointer only.
> 
> No, it is defined in our case in linux-user/qemu.h, and associated
> comment is:
> 
> /* Unlock an area of guest memory.  The first LEN bytes must be
>flushed back to guest memory. host_ptr = NULL is explicitly
>allowed and does nothing. */
> 
>>
>>>> +if (host_ver->desc_len) {
>>>> +host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc,
>>>> +   target_ver->desc_len, 0);
>>>> +if (!host_ver->desc) {
>>>> +goto err;
>>>> +}
>>>> +}
>>>> +
>>>> +unlock_user_struct(target_ver, target_addr, 0);
>>>> +return 0;
>>>> +err:
>>>> +unlock_drm_version(host_ver);
>>>> +unlock_user_struct(target_ver, target_addr, 0);
>>>> +return -ENOMEM;
>>>
>>> In fact it should be -TARGET_EFAULT: it has failed because of access rights.
>>>
>>
>> As far as I know, the lock_user is defined in
>> include/exec/softmmu-semi.h. If the parameter 'copy' is 0 (in our case),
>> lock_user will only malloc a host pointer and return it.
> 
> No, in linux-user/qemu.h:
> 
> /* Lock an area of guest memory into the host.  If copy is true then the
>host area will have the same contents as the guest.  */
> 
>> In our case, I guess the only failure from malloc() is "no memory".
> 
> See use-cases in syscall.c, they all fail with -TARGET_EFAULT.





Re: [PATCH v5] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-06-03 Thread Chen Gang
On 2020/6/3 下午5:49, Laurent Vivier wrote:
> Le 03/06/2020 à 03:08, cheng...@emindsoft.com.cn a écrit :
>> +#ifdef HAVE_DRM_H
>> +
>> +static void unlock_drm_version(struct drm_version *host_ver)
>> +{
>> +if (host_ver->name) {
>> +unlock_user(host_ver->name, 0UL, 0);
> 
> unlock_user() allows to have a NULL host pointer parameter, so you don't
> need to check. But you must provide the target pointer, with the length.
> The same below.
> 

As far as I know, the unlock_user is defined in
include/exec/softmmu-semi.h, which only checks the len before calling
cpu_memory_rw_debug, and only calls free() for the host pointer.

So we have to be sure that the host pointer must be valid. When we pass
0 length to unlock_user, we want it to free host pointer only.

>> +if (host_ver->desc_len) {
>> +host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc,
>> +   target_ver->desc_len, 0);
>> +if (!host_ver->desc) {
>> +goto err;
>> +}
>> +}
>> +
>> +unlock_user_struct(target_ver, target_addr, 0);
>> +return 0;
>> +err:
>> +unlock_drm_version(host_ver);
>> +unlock_user_struct(target_ver, target_addr, 0);
>> +return -ENOMEM;
> 
> In fact it should be -TARGET_EFAULT: it has failed because of access rights.
> 

As far as I know, the lock_user is defined in
include/exec/softmmu-semi.h. If the parameter 'copy' is 0 (in our case),
lock_user will only malloc a host pointer and return it.

In our case, I guess the only failure from malloc() is "no memory".

>> +}
>> 
>> +static inline abi_long host_to_target_drmversion(abi_ulong target_addr,
>> + struct drm_version 
>> *host_ver)
>> +{
>> +struct target_drm_version *target_ver;
>> +
>> +if (!lock_user_struct(VERIFY_WRITE, target_ver, target_addr, 0)) {
> 
> I think you should not unlock_struct() in target_to_host_drmversion() so
> you don't have to lock it again here.
> 

OK, thanks.

>> +static abi_long do_ioctl_drm(const IOCTLEntry *ie, uint8_t *buf_temp,
>> + int fd, int cmd, abi_long arg)
>> +{
>> +struct drm_version *ver;
>> +abi_long ret;
>> +
>> +switch (ie->host_cmd) {
>> +case DRM_IOCTL_VERSION:
>> +ver = (struct drm_version *)buf_temp;
> 
> you should lock the structure here (rather than in
> target_to_host_drmversion())...
> 

OK, thanks.

>> +ret = target_to_host_drmversion(ver, arg);
>> +if (is_error(ret)) {
>> +return ret;
>> +}
>> +ret = get_errno(safe_ioctl(fd, ie->host_cmd, ver));
>> +if (is_error(ret)) {
>> +unlock_drm_version(ver);
>> +return ret;
>> +}
>> +return host_to_target_drmversion(arg, ver);
> 
> and unlock the structure here (rather than in host_to_target_drmversion()).
> 
> You should return "ret" too.
> 

OK, thanks.

>> +}
>> +return -TARGET_EFAULT;
> 
> Why -TARGET_EFAULT? -TARGET_ENOSYS would be better.
> 

OK, thanks.

Chen Gang.





Re: [PATCH v4] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-06-02 Thread Chen Gang
On 2020/6/2 下午9:40, Laurent Vivier wrote:
>> +static inline abi_long target_to_host_drmversion(struct drm_version 
>> *host_ver,
>> +abi_long target_addr)
>> +{
>> +struct target_drm_version *target_ver;
>> +
>> +if (!lock_user_struct(VERIFY_READ, target_ver, target_addr, 0)) {
>> +return -TARGET_EFAULT;
>> +}
>> +__get_user(host_ver->name_len, _ver->name_len);
>> +host_ver->name = host_ver->name_len ? g2h(target_ver->name) : NULL;
>> +__get_user(host_ver->date_len, _ver->date_len);
>> +host_ver->date = host_ver->date_len ? g2h(target_ver->date) : NULL;
>> +__get_user(host_ver->desc_len, _ver->desc_len);
>> +host_ver->desc = host_ver->desc_len ? g2h(target_ver->desc) : NULL;
> 
> but I think the string buffers must be locked and access rights must be
> checked.
> 
> So I think you should have something like:
> 
> host_ver->name = lock_user(VERIFY_WRITE, target_ver->name,
>target_ver->name_len, 0);
> ...
> 

OK, thanks.

>> +unlock_user_struct(target_ver, target_addr, 0);
>> +return 0;
>> +}
>> +
>> +static inline abi_long host_to_target_drmversion(abi_ulong target_addr,
>> + struct drm_version 
>> *host_ver)
>> +{
>> +struct target_drm_version *target_ver;
>> +
>> +if (!lock_user_struct(VERIFY_WRITE, target_ver, target_addr, 0)) {
>> +return -TARGET_EFAULT;
>> +}
>> +__put_user(host_ver->version_major, _ver->version_major);
>> +__put_user(host_ver->version_minor, _ver->version_minor);
>> +__put_user(host_ver->version_patchlevel, 
>> _ver->version_patchlevel);
>> +__put_user(host_ver->name_len, _ver->name_len);
> 
> unlock_user(host_ver->name, target_ver->name, host_ver->name_len);
> ...
> 

OK, thanks.

I'll send patch v5.





Re: [PATCH v3] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-05-30 Thread Chen Gang
On 2020/5/28 下午6:25, Laurent Vivier wrote:
>> diff --git a/configure b/configure
>> index e225a1e3ff..2c2c489d1e 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3912,6 +3912,24 @@ EOF
>>  fi
>>  fi
>>  
>> +#
>> +# libdrm check
>> +
>> +cat > $TMPC << EOF
>> +#include 
>> +#include 
>> +int main(void)
>> +{
>> +struct drm_version version;
>> +ioctl(-1, DRM_IOCTL_VERSION, );
>> +return 0;
>> +}
>> +EOF
>> +if ! compile_prog "" ; then
>> +error_exit "libdrm check failed" \
>> +"Make sure to have the libdrm/drm.h installed."
>> +fi
> 
> You break the build of qemu if libdrm is not available, not a good idea.
> 
> In fact, you should only check for the include with something like
> "check_include libdrm/drm.h" and then define a HAVE_DRM_H to use it
> around the new code:
> 
> #ifdef HAVE_DRM_H
> #include 
> #endif
> 
> ...
> #ifdef HAVE_DRM_H
> static inline abi_long target_to_host_drmversion(...
> ...
> #endif
> ...
> 
> #ifdef HAVE_DRM_H
> IOCTL_SPECIAL(DRM_IOCTL_VERSION,...
> ...
> #endif
> 

OK, thanks. I'll send patch v4

Chen Gang





Re: [PATCH v2] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-05-17 Thread Chen Gang
On 2020/5/12 上午2:43, Laurent Vivier wrote:
>>  
>> +  IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm,
>> +MK_PTR(MK_STRUCT(STRUCT_drm_version)))
> 
> Add a blank line here.
>

OK, thanks.

>>  #ifdef TARGET_TIOCSTART
>>IOCTL_IGNORE(TIOCSTART)
>>IOCTL_IGNORE(TIOCSTOP)
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 8d27d10807..2eb7c91ab4 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -112,6 +112,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
> 
> I think you should check in configure that this file is available on the
> system.
> 

OK, I'll check in configure in patch v3.

>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index 152ec637cb..3c261cff0e 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -1167,6 +1167,9 @@ struct target_rtc_pll_info {
>>  #define TARGET_DM_TARGET_MSG  TARGET_IOWRU(0xfd, 0x0e)
>>  #define TARGET_DM_DEV_SET_GEOMETRYTARGET_IOWRU(0xfd, 0x0f)
>>  
>> +/* drm ioctls */
>> +#define TARGET_DRM_IOCTL_VERSION  TARGET_IOWRU('d', 0x00)
> 
> Why do you use the TARGET_IOWRU variant?
> 
> Can't you use TARGET_IOWR('d', 0x00, struct target_drm_version)?
> 

Because qemu will automatically set the size with the target structure
size in syscall_init(). It'll be more easier. e.g. usb ioctls and device
mapper ioctls also do like this.

Thanks.





Re: [PATCH] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION

2020-03-12 Thread Chen Gang
Thank you very much for your reply. :)

DRM_IOCTL_VERSION has to be a special one, but my patch really need be
improved. The string pointers should be translated, but the new string
buffers need not be allocated.

I will try to send patch v2 within this week end.

DRM_IOCTL_* really have many commands, at present, I have implemented
about 20+ commands for i965 gpu using, although some of them are not
generic enough. I will try to send all of them when I have free time.

Thanks.

On 2020/3/11 下午3:35, Laurent Vivier wrote:
> Le 10/03/2020 à 21:16, Laurent Vivier a écrit :
>> Le 26/02/2020 à 12:38, cheng...@emindsoft.com.cn a écrit :
>>> From: Chen Gang 
>>>
>>> The other DRM_IOCTL_* commands will be done later.
>>>
>>> Signed-off-by: Chen Gang 
>>> ---
>>>  linux-user/ioctls.h|   3 +
>>>  linux-user/syscall.c   | 134 +
>>>  linux-user/syscall_defs.h  |  16 +
>>>  linux-user/syscall_types.h |  12 
>>>  4 files changed, 165 insertions(+)
>>>
>>> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
>>> index 0defa1d8c1..c2294b48a0 100644
>>> --- a/linux-user/ioctls.h
>>> +++ b/linux-user/ioctls.h
>>> @@ -574,6 +574,9 @@
>>>IOCTL_SPECIAL(SIOCDELRT, IOC_W, do_ioctl_rt,
>>>  MK_PTR(MK_STRUCT(STRUCT_rtentry)))
>>>  
>>> +  IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm,
>>> +MK_PTR(MK_STRUCT(STRUCT_drm_version)))
>>> +
>>
>> Rather than adding a specific function to process the structure, perhaps
>> we can add this in a generic way?
>>
>> The problem with drm_version structure is the pointers to the strings.
>>
>> Did you try to add a TYPE_STRING in
>> thunk_type_size()/thunk_type_align()/think_convert()/do_ioctl() to do that?
> 
> In fact we can't do that because we need to know the size of the buffer
> to allocate and it is provided by another field. It cannot be generic,
> so I think what you do is the best we can do.
> 
> Thanks,
> LAurent
> 
> 
> 
> 
> 





Re: [PATCH] target: i386: Check float overflow about register stack

2020-02-24 Thread Chen Gang
On 2020/2/24 下午8:43, Paolo Bonzini wrote:
> On 22/02/20 13:25, Chen Gang wrote:
>> On 2020/2/22 下午3:37, Paolo Bonzini wrote:
>>> On 22/02/20 03:10, Chen Gang wrote:
>>>> Set C1 to 1 if stack overflow occurred; set to 0 otherwise".
>>>>
>>>> In helper_fxam_ST0, I guess, we need "env->fpus |= 0x200" (but I don't
>>>> know wheter it will be conflict with SIGND(temp)). And we have to still
>>>> need foverflow, because all env->fptags being 0 doesn't mean overflow.
>>>
>>> No, you need to add "env->fpus |= 0x200" and "env->fpus &= ~0x200"
>>> directly to fpush, fpop, etc.
>>>
>>
>> OK. The content below is my next TODO, welcome your opinions.
>>
>> When overflow occurs, for me, we need keep everything no touch except
>> set C1 flag.
> 
> No, push will overwrite the top entry if there is overflow.
> 
>> In fxam, we don't clear C1, but keep no touch for clearning
>> C1 in another places.
> 
> FXAM is neither push nor pop, it just detects an empty slot via fptags.
>  FXAM should be okay with my patch.
> 

OK. I am not quite clear about it, but it fixes the current issues at
least. Please apply your patch.

Thanks.





Re: [PATCH] target: i386: Check float overflow about register stack

2020-02-22 Thread Chen Gang
On 2020/2/22 下午3:37, Paolo Bonzini wrote:
> On 22/02/20 03:10, Chen Gang wrote:
>> Set C1 to 1 if stack overflow occurred; set to 0 otherwise".
>>
>> In helper_fxam_ST0, I guess, we need "env->fpus |= 0x200" (but I don't
>> know wheter it will be conflict with SIGND(temp)). And we have to still
>> need foverflow, because all env->fptags being 0 doesn't mean overflow.
> 
> No, you need to add "env->fpus |= 0x200" and "env->fpus &= ~0x200"
> directly to fpush, fpop, etc.
> 

OK. The content below is my next TODO, welcome your opinions.

When overflow occurs, for me, we need keep everything no touch except
set C1 flag. In fxam, we don't clear C1, but keep no touch for clearning
C1 in another places.

When underflow occurs, for me, we need keep everything no touch except
set env->fpstt 8, so the next consecutive fpop/f[i]stp* can be checked
easier, and the next fpush/f[i]ld* can work well in normal.

For fxam, we check env->fpstt == 8 and env->fptags for empty. And when
env->fpstt is 8, it need be set 7 before used in fincstp and ffree_STN.

Thanks.





Re: [PATCH] target: i386: Check float overflow about register stack

2020-02-21 Thread Chen Gang
On 2020/2/22 上午10:10, Chen Gang wrote:
> On 2020/2/22 上午12:18, Paolo Bonzini wrote:
>> On 21/02/20 15:09, Chen Gang wrote:
>>>> -/* XXX: test fptags too */
>>>> +if (env->fptags[env->fpstt]) {
>>>> +env->fpus |= 0x4100; /* Empty */
>>>> +return;
>>>> +}
>>>> +
>>> For fpop overflow, this fix is enough, but for me, we still need
>>> foverflow to check fpush/fld*_ST0 overflow.
>>>
>>> Don't you think we need check fpush/f[i]ld*_ST0 overflow?
>>
>> After fld/fild or any other push, FXAM ST0 should not return empty in my
>> opinion.
>>
> 
> OK, it sounds reasonable.
> 
> After check the intel document for f[i]ld* instructions, it says:
> 
>   "Set C1 to 1 if stack overflow occurred; set to 0 otherwise".
> 
> In helper_fxam_ST0, I guess, we need "env->fpus |= 0x200" (but I don't
> know wheter it will be conflict with SIGND(temp)). And we have to still
> need foverflow, because all env->fptags being 0 doesn't mean overflow.
> 

After read the document more about fstp* and fld* instructions, I find
that fstp* are also related with C1, and fld* will generate IS exception
when stack underflow or overflow occurred.

It seems more modifications should be done (but they will be several
patches).

Thanks.





Re: [PATCH] target: i386: Check float overflow about register stack

2020-02-21 Thread Chen Gang
On 2020/2/22 上午12:18, Paolo Bonzini wrote:
> On 21/02/20 15:09, Chen Gang wrote:
>>> -/* XXX: test fptags too */
>>> +if (env->fptags[env->fpstt]) {
>>> +env->fpus |= 0x4100; /* Empty */
>>> +return;
>>> +}
>>> +
>> For fpop overflow, this fix is enough, but for me, we still need
>> foverflow to check fpush/fld*_ST0 overflow.
>>
>> Don't you think we need check fpush/f[i]ld*_ST0 overflow?
> 
> After fld/fild or any other push, FXAM ST0 should not return empty in my
> opinion.
> 

OK, it sounds reasonable.

After check the intel document for f[i]ld* instructions, it says:

  "Set C1 to 1 if stack overflow occurred; set to 0 otherwise".

In helper_fxam_ST0, I guess, we need "env->fpus |= 0x200" (but I don't
know wheter it will be conflict with SIGND(temp)). And we have to still
need foverflow, because all env->fptags being 0 doesn't mean overflow.

Thanks.





Re: [PATCH] target: i386: Check float overflow about register stack

2020-02-21 Thread Chen Gang
On 2020/2/21 下午4:58, Paolo Bonzini wrote:
> On 21/02/20 04:45, cheng...@emindsoft.com.cn wrote:
>>  static inline void fpush(CPUX86State *env)
>>  {
>> -env->fpstt = (env->fpstt - 1) & 7;
>> -env->fptags[env->fpstt] = 0; /* validate stack entry */
>> +set_fpstt(env, env->fpstt - 1, false, true);
> 
> On overflow fpstt is ~0, so this does:
> 
> env->foverflow = true;
> env->fpstt = 7;
> env->fptags[7] = 0;  /* validate stack entry */
> 
> Is this correct?  You are going to set ST0 so the register should not be
> marked empty.
> 

Originally, I wanted to add foverflow to mark the stack overflow only,
and kept another things no touch.

But I think what you said above is correct, for me, if fpush/f[i]ld*_STO
are overflow, the env->fpstt, env->fpregs and env->fptags should be kept
no touch, and foverflow is set true, so there is no negative effect.

Welcome your idea.

>>  void helper_fdecstp(CPUX86State *env)
>>  {
>> -env->fpstt = (env->fpstt - 1) & 7;
>> +set_fpstt(env, env->fpstt - 1, false, false);
> 
> This is clearing env->foverflow.  But after 8 consecutive fdecstp or
> fincstp the result of FXAM should not change.
> 
>>  env->fpus &= ~0x4700;
>>  }
>>  
>>  void helper_fincstp(CPUX86State *env)
>>  {
>> -env->fpstt = (env->fpstt + 1) & 7;
>> +set_fpstt(env, env->fpstt + 1, true, false);
> 
> Same here.
> 

OK. thanks.

Now if foverflow is only for fpush/f[i]ld*_ST0, I guess fincstp/fdecstp
can clear foverflow. The env->fptags are only for fpop, which keep no
touch in fincstp/fdecstp.

> The actual bug is hinted in helper_fxam_ST0:
> 
> /* XXX: test fptags too */
> 
> I think the correct fix should be something like
> 
> diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
> index 99f28f267f..792a128a6d 100644
> --- a/target/i386/fpu_helper.c
> +++ b/target/i386/fpu_helper.c
> @@ -991,7 +991,11 @@ void helper_fxam_ST0(CPUX86State *env)
>  env->fpus |= 0x200; /* C1 <-- 1 */
>  }
> 
> -/* XXX: test fptags too */
> +if (env->fptags[env->fpstt]) {
> +env->fpus |= 0x4100; /* Empty */
> +return;
> +}
> +

For fpop overflow, this fix is enough, but for me, we still need
foverflow to check fpush/fld*_ST0 overflow.

Don't you think we need check fpush/f[i]ld*_ST0 overflow?

Thanks





Re: [Qemu-devel] Deprecate tilegx ?

2018-03-13 Thread Chen Gang

On 2/28/18 20:41, Bastian Koppelmann wrote:
> On 02/28/2018 07:11 AM, Thomas Huth wrote:
>> On 27.02.2018 12:51, Peter Maydell wrote:
>>
>>> Possibly there are other target architectures we could reasonably
>>> deprecate-and-remove (though none of the other ones Linux is dropping
>>> in this round are ones we support)...
>>
>> I'd vote for marking tilegx as deprecated, too, since we even do not
>> have an active maintainer for that CPU core (at least I did not spot one
>> in our MAINTAINERS file). Opinions?
> 
> I always saw it as a big plus that QEMU supports nearly any
> architecture, no matter how obscure it is. So I'm a bit more hesitant on
> dropping architectures quickly.
> 

Firstly, sorry for too long time to no response about tilegx.

During latest 2 years, I turned to android development (custom aosp with
java/c++/c, merge android and linux together), and revert engineering
about android (java/c).

At present, sorry, I really have no time for tilegx any more. :(

For me, tilegx is still a good platform, and we have done much things,
now, so if we can find another maintainers/volunteers for it, I hope
qemu can still support tilegx.

By the way, for floating point instructions patches, I have sent before
(2 years ago), it seems I did not get any reply. I hope they are useful
for tilegx.

Thanks.
-- 
Chen Gang (陈刚)




Re: [Qemu-devel] [PATCH 16/19] target-tilegx: move cpu_exec_realize() to realize function

2016-10-05 Thread Chen Gang

On 10/6/16 03:38, Laurent Vivier wrote:
> I've removed the cannot_destroy_with_object_finalize_yet field as
> cpu_exec_init() is not called by tilegx_cpu_initfn() anymore
> (not tested with QOM command as tilegx is only available in linux-user
>  mode)
> 


For master branch, I can not find cpu_exec_realize in total source code,
I am not quite sure whether it is ok or not.

And in honest, now, tilegx-linux-user is not implemented completely, the
floating point insns are not supported by our main branch (which should
be one of main feature for tilegx) -- which I should try.

Thanks.

> CC: Chen Gang <gang.chen.5...@gmail.com>
> Signed-off-by: Laurent Vivier <lviv...@redhat.com>
> ---
>  target-tilegx/cpu.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
> index f7ec920..6be69ef 100644
> --- a/target-tilegx/cpu.c
> +++ b/target-tilegx/cpu.c
> @@ -92,6 +92,13 @@ static void tilegx_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>  {
>  CPUState *cs = CPU(dev);
>  TileGXCPUClass *tcc = TILEGX_CPU_GET_CLASS(dev);
> +Error *local_err = NULL;
> +
> +cpu_exec_realize(cs, _err);
> +if (local_err != NULL) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  
>  cpu_reset(cs);
>  qemu_init_vcpu(cs);
> @@ -108,7 +115,6 @@ static void tilegx_cpu_initfn(Object *obj)
>  
>  cs->env_ptr = env;
>  cpu_exec_init(cs, _abort);
> -cpu_exec_realize(cs, _abort);
>  
>  if (tcg_enabled() && !tcg_initialized) {
>  tcg_initialized = true;
> @@ -163,13 +169,6 @@ static void tilegx_cpu_class_init(ObjectClass *oc, void 
> *data)
>  cc->set_pc = tilegx_cpu_set_pc;
>  cc->handle_mmu_fault = tilegx_cpu_handle_mmu_fault;
>  cc->gdb_num_core_regs = 0;
> -
> -/*
> - * Reason: tilegx_cpu_initfn() calls cpu_exec_init(), which saves
> - * the object in cpus -> dangling pointer after final
> - * object_unref().
> - */
> -dc->cannot_destroy_with_object_finalize_yet = true;
>  }
>  
>  static const TypeInfo tilegx_cpu_type_info = {
> 

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH v6 0/5] target-tilegx: Implement floating point instructions

2016-06-05 Thread Chen Gang
Hello Maintainers:

Please help check this patch when you have time.

Thanks.

On 5/15/16 07:40, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang <cheng...@emindsoft.com.cn>
> 
> These patches are the normal floating point implementation, instead of
> the original temporary one.
> 
> It passes building, and gcc testsuite. 
> 
> 
> Chen Gang (5):
>   fpu: softfloat: Add normalize_roundpack_float[32|64] functions
>   target-tilegx/helper-fshared.h: Add floating point shared function
>   target-tilegx/helper-fsingle.c: Implement single floating point
>   target-tilegx/helper-fdouble.c: Implement double floating point
>   target-tilegx: Integrate floating pointer implementation
> 
>  fpu/softfloat.c|  65 
>  include/fpu/softfloat.h|  15 ++
>  target-tilegx/Makefile.objs|   3 +-
>  target-tilegx/helper-fdouble.c | 365 
> +
>  target-tilegx/helper-fshared.h |  56 +++
>  target-tilegx/helper-fsingle.c | 201 +++
>  target-tilegx/helper.h |  12 ++
>  target-tilegx/translate.c  |  66 +++-
>  8 files changed, 773 insertions(+), 10 deletions(-)
>  create mode 100644 target-tilegx/helper-fdouble.c
>  create mode 100644 target-tilegx/helper-fshared.h
>  create mode 100644 target-tilegx/helper-fsingle.c
> 

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use target address instead of host address for microblaze restorer

2016-05-13 Thread Chen Gang
On 5/6/16 00:11, Edgar E. Iglesias wrote:
> On Thu, May 05, 2016 at 10:48:57PM +0800, Chen Gang wrote:
>> On 5/5/16 00:05, Peter Maydell wrote:
>>> On 29 March 2016 at 15:13,  <cheng...@emindsoft.com.cn> wrote:
>>>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>>>
>>>> The return address is in target space, so the restorer address needs to
>>>> be target space, too.
>>>>
>>>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>>>> ---
>>>>  linux-user/signal.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>>>> index 4157154..c0a6f7e 100644
>>>> --- a/linux-user/signal.c
>>>> +++ b/linux-user/signal.c
>>>> @@ -3532,7 +3532,8 @@ static void setup_frame(int sig, struct 
>>>> target_sigaction *ka,
>>>>
>>>>  /* Return from sighandler will jump to the tramp.
>>>> Negative 8 offset because return is rtsd r15, 8 */
>>>> -env->regs[15] = ((unsigned long)frame->tramp) - 8;
>>>> +env->regs[15] = frame_addr + offsetof(struct target_signal_frame, 
>>>> tramp)
>>>> +   - 8;
>>>>  }
>>>>
>>>>  /* Set up registers for signal handler */
>>>
>>> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>>>
>>
>> Thank all of you for the 2 patches reviewing.
>>
>> I guess, this month, I may have free time (at least, will not be as busy
>> as the previous month), I shall finish tilegx floating point insns (it
>> has been delayed too long).
> 
> Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> 

OK, thank you for your work.

And now (finally), I guess, I should have free time for open source, and
I should try to finish tilegx floating point insns within this month.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use target address instead of host address for microblaze restorer

2016-05-05 Thread Chen Gang
On 5/5/16 00:05, Peter Maydell wrote:
> On 29 March 2016 at 15:13,  <cheng...@emindsoft.com.cn> wrote:
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> The return address is in target space, so the restorer address needs to
>> be target space, too.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/signal.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index 4157154..c0a6f7e 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -3532,7 +3532,8 @@ static void setup_frame(int sig, struct 
>> target_sigaction *ka,
>>
>>  /* Return from sighandler will jump to the tramp.
>> Negative 8 offset because return is rtsd r15, 8 */
>> -env->regs[15] = ((unsigned long)frame->tramp) - 8;
>> +env->regs[15] = frame_addr + offsetof(struct target_signal_frame, 
>> tramp)
>> +   - 8;
>>  }
>>
>>  /* Set up registers for signal handler */
> 
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
> 

Thank all of you for the 2 patches reviewing.

I guess, this month, I may have free time (at least, will not be as busy
as the previous month), I shall finish tilegx floating point insns (it
has been delayed too long).


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH v2] Use frame->retcode instead of frame address for alpha target restorer

2016-04-17 Thread Chen Gang
Hello all:

I am very sorry for delay finishing tilegx floating points insns.

The reason is that during these days (I guess, until 2016-05-01), I have
no any free time.

 - During these days, I am often not at home (sleep in a hotel near the
   office), and often work in the weekend.

 - It is about the android frameworks/base, which is mostly written in
   Java. It is of cause very simple to me, but really quite much work
   need be done.

 - And I guess, after 2016-05-01, I have enough free time resources on
   our tilegx floating point insns.

And sorry again for my delaying, if we can not bear the time point,
please help implement the tilegx floating point insns.

Thanks.

On 3/31/16 21:57, Chen Gang wrote:
> 
> On 3/31/16 00:09, Laurent Vivier wrote:
>>
>> Le 30/03/2016 17:42, cheng...@emindsoft.com.cn a écrit :
>>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>>
>>> The restorer needs the return code address which is frame->retcode, not
>>> frame itself.
>>>
>>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>>
>> Reviewed-by: Laurent Vivier <laur...@vivier.eu>
>>
> 
> Thank all of you for your reviewing.
> 
> Next, I shall try to finish tilegx floating point insns.
> 
> Thanks.
> 

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH v2] Use frame->retcode instead of frame address for alpha target restorer

2016-03-31 Thread Chen Gang

On 3/31/16 00:09, Laurent Vivier wrote:
> 
> Le 30/03/2016 17:42, cheng...@emindsoft.com.cn a écrit :
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> The restorer needs the return code address which is frame->retcode, not
>> frame itself.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> 
> Reviewed-by: Laurent Vivier <laur...@vivier.eu>
> 

Thank all of you for your reviewing.

Next, I shall try to finish tilegx floating point insns.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use frame->retcode instead of frame address for alpha target restorer

2016-03-29 Thread Chen Gang
On 3/29/16 22:25, Laurent Vivier wrote:
> Le 29/03/2016 16:01, cheng...@emindsoft.com.cn a écrit :
>> The restorer needs the return code address which is frame->retcode, not
>> frame itself.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/signal.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index e487f9e..4157154 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -5455,7 +5455,7 @@ static void setup_rt_frame(int sig, struct 
>> target_sigaction *ka,
>> >retcode[1]);
>>  __put_user(INSN_CALLSYS, >retcode[2]);
>>  /* imb(); */
>> -r26 = frame_addr;
>> +r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode);
>>  }
>>  
>>  if (err) {
>>
> 
> If you change setup_rt_frame(), you must update setup_frame() too.
> 

Oh, yes, thanks.

> It seems correct.
> 
> Richard, as you have written the original code, could you check this is
> correct?
> 

Please give a check when you have time.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH] linux-user/signal.c: Generate opcode data for restorer in setup_rt_frame

2016-03-29 Thread Chen Gang
On 3/29/16 06:57, Chen Gang wrote:
> On 3/29/16 06:17, Laurent Vivier wrote:
>>
>> The address of retcode in host and guest can differ.
>> You need something like:
>>
>>  restorer = (unsigned long)(frame_addr + offsetof(struct
>> target_rt_sigframe, retcode));
>>
>> I've experienced this on sh4 (see commit 2a0fa68)
>>
> 
> OK, thanks. What you said above sounds reasonable to me. :-)
> 
> I shall send patch v2 for it (although tilegx is a pure 64-bit target,
> with this patch, I guess, tilegx target should still run correctly under
> 32-bit host).
> 
> By the way, it looks that s390x and microblaze targets also have the
> same issue.

It looks the alpha target may also need consider about retcode, for "r26
= frame_addr" in setup_rt_frame().

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH] linux-user/signal.c: Generate opcode data for restorer in setup_rt_frame

2016-03-28 Thread Chen Gang
On 3/29/16 06:17, Laurent Vivier wrote:
>> On 3/15/16 05:51, cheng...@emindsoft.com.cn wrote:
>>>
>>> Original implementation uses do_rt_sigreturn directly in host space,
>>> when a guest program is in unwind procedure in guest space, it will get
>>> an incorrect restore address, then causes unwind failure.
>>>
>>> Also cleanup the original incorrect indentation.
>>>
>>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>>> ---
>>>  linux-user/signal.c | 12 ++--
>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>>> index 919aa83..0e3b1c6 100644
>>> --- a/linux-user/signal.c
>>> +++ b/linux-user/signal.c
>>> @@ -5566,8 +5566,13 @@ struct target_rt_sigframe {
>>>  unsigned char save_area[16]; /* caller save area */
>>>  struct target_siginfo info;
>>>  struct target_ucontext uc;
>>> +abi_ulong retcode[2];
>>>  };
>>>  
>>> +#define INSN_MOVELI_R10_139  0x00045fe551483000ULL /* { moveli r10, 139 } 
>>> */
>>> +#define INSN_SWINT1  0x286b180051485000ULL /* { swint1 } */
>>> +
>>> +
>>>  static void setup_sigcontext(struct target_sigcontext *sc,
>>>   CPUArchState *env, int signo)
>>>  {
>>> @@ -5643,9 +5648,12 @@ static void setup_rt_frame(int sig, struct 
>>> target_sigaction *ka,
>>>  __put_user(target_sigaltstack_used.ss_size, 
>>> >uc.tuc_stack.ss_size);
>>>  setup_sigcontext(>uc.tuc_mcontext, env, info->si_signo);
>>>  
>>> -restorer = (unsigned long) do_rt_sigreturn;
>>>  if (ka->sa_flags & TARGET_SA_RESTORER) {
>>> -restorer = (unsigned long) ka->sa_restorer;
>>> +restorer = (unsigned long) ka->sa_restorer;
>>> +} else {
>>> +__put_user(INSN_MOVELI_R10_139, >retcode[0]);
>>> +__put_user(INSN_SWINT1, >retcode[1]);
>>> +restorer = (unsigned long)frame->retcode;
> 
> The address of retcode in host and guest can differ.
> You need something like:
> 
>   restorer = (unsigned long)(frame_addr + offsetof(struct
> target_rt_sigframe, retcode));
> 
> I've experienced this on sh4 (see commit 2a0fa68)
> 

OK, thanks. What you said above sounds reasonable to me. :-)

I shall send patch v2 for it (although tilegx is a pure 64-bit target,
with this patch, I guess, tilegx target should still run correctly under
32-bit host).

By the way, it looks that s390x and microblaze targets also have the
same issue.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH] target-tilegx: Support dumping working flow instructions and registers

2016-03-27 Thread Chen Gang
Hello all:

Is this patch helpful? If it shouldn't be merged into our master branch,
please let me know, I shall treat it as my own local patch (when I
update my local master branch, I'll merge it at last).

Thanks.

On 3/16/16 23:51, Chen Gang wrote:
> Hello all:
> 
> It is only for analyzing issues, I guess it is helpful, so I send it,
> welcome any other ideas, suggestions, and completions.
> 
> And next, I shall continue to improve the floating point instruction
> features: "remove (u)int64_to_float64 from fdouble implementation".
> 
> Thanks.
> 
> On 3/16/16 23:38, cheng...@emindsoft.com.cn wrote:
>> From: Chen Gang <gang.chen.5...@gmail.com>
>>
>> It is only for debug and analyzing tilegx qemu related issues.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  target-tilegx/helper.c| 34 ++
>>  target-tilegx/helper.h|  2 ++
>>  target-tilegx/translate.c | 37 +
>>  3 files changed, 73 insertions(+)
>>
>> diff --git a/target-tilegx/helper.c b/target-tilegx/helper.c
>> index 616c5c7..c7cfe66 100644
>> --- a/target-tilegx/helper.c
>> +++ b/target-tilegx/helper.c
>> @@ -160,3 +160,37 @@ uint64_t helper_cmul2(uint64_t srca, uint64_t srcb, int 
>> shift, int round)
>>  
>>  return deposit32(realr >> shift, 16, 16, imagr >> shift);
>>  }
>> +
>> +extern char **debug_buf;
>> +
>> +#define DEBUG_DUMP_REGS
>> +
>> +static void dump_regs(CPUTLGState *env, int idx)
>> +{
>> +#ifdef DEBUG_DUMP_REGS
>> +int i;
>> +
>> +if (debug_buf[idx][2] == '\n') {
>> +for (i = 0; i < TILEGX_R_COUNT; i++) {
>> +if (!(i % 4)) {
>> +fprintf(stderr, "\n");
>> +}
>> +fprintf(stderr, "r%2.2u = %16.16lx, ", i, env->regs[i]);
>> +}
>> +fprintf(stderr, "\n");
>> +for (i = 0; i < TILEGX_SPR_COUNT; i++) {
>> +if (!(i % 8)) {
>> +fprintf(stderr, "\n");
>> +}
>> +fprintf(stderr, "s%2.2u = %16.16lx, ", i, env->spregs[i]);
>> +}
>> +fprintf(stderr, "\n");
>> +}
>> +#endif
>> +}
>> +
>> +void helper_print_step(CPUTLGState *env, int idx)
>> +{
>> +fprintf(stderr, "%s", debug_buf[idx]);
>> +dump_regs(env, idx);
>> +}
>> diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
>> index 9281d0f..74f796a 100644
>> --- a/target-tilegx/helper.h
>> +++ b/target-tilegx/helper.h
>> @@ -24,3 +24,5 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, 
>> i64)
>>  DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>>  DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>>  DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>> +
>> +DEF_HELPER_2(print_step, void, env, int)
>> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
>> index 03918eb..8489494 100644
>> --- a/target-tilegx/translate.c
>> +++ b/target-tilegx/translate.c
>> @@ -47,6 +47,43 @@ static const char * const reg_names[64] = {
>>  "sn", "idn0", "idn1", "udn0", "udn1", "udn2", "udn2", "zero"
>>  };
>>  
>> +#define STATIC_DEBUG
>> +
>> +char **debug_buf;
>> +
>> +#ifdef STATIC_DEBUG
>> +
>> +#include 
>> +
>> +#define qemu_log_mask(flag, fmt, ...)  debug_print(flag, fmt, ##__VA_ARGS__)
>> +
>> +static int debug_idx;
>> +
>> +extern void debug_print(int flag, const char *fmt, ...);
>> +
>> +void debug_print(int flag, const char *fmt, ...)
>> +{
>> +va_list args;
>> +char buf[0x400];
>> +TCGv_i32 tmp;
>> +
>> +va_start(args, fmt);
>> +vsprintf(buf, fmt, args);
>> +va_end(args);
>> +
>> +if (!debug_buf) {
>> +debug_buf = malloc(sizeof(char *) * 0x40);
>> +}
>> +debug_buf[debug_idx] = malloc(strlen(buf) + 1);
>> +strcpy(debug_buf[debug_idx], buf);
>> +
>> +tmp = tcg_const_i32(debug_idx++);
>> +gen_helper_print_step(cpu_env, tmp);
>> +tcg_temp_free_i32(tmp);
>> +}
>> +
>> +#endif
>> +
>>  /* Modified registers are cached in temporaries until the end of the 
>> bundle. */
>>  typedef struct {
>>  unsigned reg;
>>
> 

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH] linux-user/signal.c: Generate opcode data for restorer in setup_rt_frame

2016-03-27 Thread Chen Gang
Hello All:

Please help check this patch when you have time.

After this patch, we can let gcc testsuite cleanup-10 run successfully.

Next, I shall continue to implement floating point instructions: remove
(u)int64_to_float64 from fdouble implementation.


Thanks.

On 3/15/16 05:51, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang <cheng...@emindsoft.com.cn>
> 
> Original implementation uses do_rt_sigreturn directly in host space,
> when a guest program is in unwind procedure in guest space, it will get
> an incorrect restore address, then causes unwind failure.
> 
> Also cleanup the original incorrect indentation.
> 
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
>  linux-user/signal.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 919aa83..0e3b1c6 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -5566,8 +5566,13 @@ struct target_rt_sigframe {
>  unsigned char save_area[16]; /* caller save area */
>  struct target_siginfo info;
>  struct target_ucontext uc;
> +abi_ulong retcode[2];
>  };
>  
> +#define INSN_MOVELI_R10_139  0x00045fe551483000ULL /* { moveli r10, 139 } */
> +#define INSN_SWINT1  0x286b180051485000ULL /* { swint1 } */
> +
> +
>  static void setup_sigcontext(struct target_sigcontext *sc,
>   CPUArchState *env, int signo)
>  {
> @@ -5643,9 +5648,12 @@ static void setup_rt_frame(int sig, struct 
> target_sigaction *ka,
>  __put_user(target_sigaltstack_used.ss_size, 
> >uc.tuc_stack.ss_size);
>  setup_sigcontext(>uc.tuc_mcontext, env, info->si_signo);
>  
> -restorer = (unsigned long) do_rt_sigreturn;
>  if (ka->sa_flags & TARGET_SA_RESTORER) {
> -restorer = (unsigned long) ka->sa_restorer;
> +restorer = (unsigned long) ka->sa_restorer;
> +} else {
> +__put_user(INSN_MOVELI_R10_139, >retcode[0]);
> +__put_user(INSN_SWINT1, >retcode[1]);
> +    restorer = (unsigned long)frame->retcode;
>  }
>  env->pc = (unsigned long) ka->_sa_handler;
>  env->regs[TILEGX_R_SP] = (unsigned long) frame;
> 

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH] target-tilegx: Support dumping working flow instructions and registers

2016-03-19 Thread Chen Gang
Hello all:

It is only for analyzing issues, I guess it is helpful, so I send it,
welcome any other ideas, suggestions, and completions.

And next, I shall continue to improve the floating point instruction
features: "remove (u)int64_to_float64 from fdouble implementation".

Thanks.

On 3/16/16 23:38, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang <gang.chen.5...@gmail.com>
> 
> It is only for debug and analyzing tilegx qemu related issues.
> 
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
>  target-tilegx/helper.c| 34 ++
>  target-tilegx/helper.h|  2 ++
>  target-tilegx/translate.c | 37 +
>  3 files changed, 73 insertions(+)
> 
> diff --git a/target-tilegx/helper.c b/target-tilegx/helper.c
> index 616c5c7..c7cfe66 100644
> --- a/target-tilegx/helper.c
> +++ b/target-tilegx/helper.c
> @@ -160,3 +160,37 @@ uint64_t helper_cmul2(uint64_t srca, uint64_t srcb, int 
> shift, int round)
>  
>  return deposit32(realr >> shift, 16, 16, imagr >> shift);
>  }
> +
> +extern char **debug_buf;
> +
> +#define DEBUG_DUMP_REGS
> +
> +static void dump_regs(CPUTLGState *env, int idx)
> +{
> +#ifdef DEBUG_DUMP_REGS
> +int i;
> +
> +if (debug_buf[idx][2] == '\n') {
> +for (i = 0; i < TILEGX_R_COUNT; i++) {
> +if (!(i % 4)) {
> +fprintf(stderr, "\n");
> +}
> +fprintf(stderr, "r%2.2u = %16.16lx, ", i, env->regs[i]);
> +}
> +fprintf(stderr, "\n");
> +for (i = 0; i < TILEGX_SPR_COUNT; i++) {
> +if (!(i % 8)) {
> +fprintf(stderr, "\n");
> +}
> +fprintf(stderr, "s%2.2u = %16.16lx, ", i, env->spregs[i]);
> +}
> +fprintf(stderr, "\n");
> +}
> +#endif
> +}
> +
> +void helper_print_step(CPUTLGState *env, int idx)
> +{
> +fprintf(stderr, "%s", debug_buf[idx]);
> +dump_regs(env, idx);
> +}
> diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
> index 9281d0f..74f796a 100644
> --- a/target-tilegx/helper.h
> +++ b/target-tilegx/helper.h
> @@ -24,3 +24,5 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, 
> i64)
>  DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>  DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>  DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
> +
> +DEF_HELPER_2(print_step, void, env, int)
> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
> index 03918eb..8489494 100644
> --- a/target-tilegx/translate.c
> +++ b/target-tilegx/translate.c
> @@ -47,6 +47,43 @@ static const char * const reg_names[64] = {
>  "sn", "idn0", "idn1", "udn0", "udn1", "udn2", "udn2", "zero"
>  };
>  
> +#define STATIC_DEBUG
> +
> +char **debug_buf;
> +
> +#ifdef STATIC_DEBUG
> +
> +#include 
> +
> +#define qemu_log_mask(flag, fmt, ...)  debug_print(flag, fmt, ##__VA_ARGS__)
> +
> +static int debug_idx;
> +
> +extern void debug_print(int flag, const char *fmt, ...);
> +
> +void debug_print(int flag, const char *fmt, ...)
> +{
> +va_list args;
> +char buf[0x400];
> +TCGv_i32 tmp;
> +
> +va_start(args, fmt);
> +vsprintf(buf, fmt, args);
> +va_end(args);
> +
> +if (!debug_buf) {
> +debug_buf = malloc(sizeof(char *) * 0x40);
> +}
> +debug_buf[debug_idx] = malloc(strlen(buf) + 1);
> +strcpy(debug_buf[debug_idx], buf);
> +
> +tmp = tcg_const_i32(debug_idx++);
> +gen_helper_print_step(cpu_env, tmp);
> +tcg_temp_free_i32(tmp);
> +}
> +
> +#endif
> +
>  /* Modified registers are cached in temporaries until the end of the bundle. 
> */
>  typedef struct {
>  unsigned reg;
> 

-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.



Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()

2016-01-28 Thread Chen Gang

On 2016年01月28日 22:54, Peter Maydell wrote:
> On 27 January 2016 at 01:37, Chen Gang <cheng...@emindsoft.com.cn> wrote:
>> Within one single call to target_mmap(), it should be OK.
>>
>> But multiple call to target_mmap(), may call mmap_frag() multiple times
>> for the same host page (also for the same target page). In our case:
>>
>>  - 4600 
>> mmap2(0x0034,135168,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0) 
>> = 0x0034
>>
>>It will call mmap_frag() with start address 0x0034 + 128KB, and
>>set the target page with PAGE_VALID. But left the half below host
>>page without PAGE_VALID.
> 
> So, just to put some numbers in here:
> 
>  0x34 .. 0x34  0x35 .. 0x35 0x36 .. 0x360fff
>(64k, first host page)   (64k, second host page)  (4k guest page)
> 
> and we call mmap_frag() once for that last 4K fragment. It should:
>  * allocate a host page (since none is there yet)
>  * return to target_mmap, which will mark the range
>0x3f .. 0x360fff as PROT_VALID (together with the other
>read/write/etc permissions)
> 
> I think this part is definitely correct.
> 

Yes to me.

>>  - 4600 mmap2(0x0034,135168,PROT_READ,MAP_SHARED|MAP_FIXED,8,0) = 
>> 0x0034
>>
>>It will call mmap_frag() with start address 0x0034 + 128KB, and
>>check the half below host page which has no PAGE_VALID, then "prot1
>>== 0", mmap_frag() thinks "no page was there, so we allocate one".
> 
> On the second call, we again call mmap_frag for that last 4K.
> We check for any other valid guest pages in the 64k host page,
> and there aren't any. This will indeed cause us to mmap() again,
> which ideally we would not. But:
> 

OK.

> (1) Is this actually causing anything to fail? Calling host
> mmap() again is ever so slightly inefficient, but I don't think
> that it causes the guest to see anything wrong.
> 

For me, something may be a little complex (assume 8KB host page, 4KB
guest page):

 - 1st mmap2() is for MAP_PRIVATE, 2nd mmap2() is for MAP_SHARED.

 - So, if 2nd call mmap_frag() with the same start address only calls
   mprotect(), doesn't call mmap2() again, the target page will be
   still MAP_PRIVATE? (but caller wants it to be MAP_SHARED).

And theoretically, if the caller wants the 2 target pages within a host
page have different mapping attributes (e.g. half top host page is
MAP_SHARED, but half bottom host page is MAP_PRIVATE):

 - I guess, our current softmmu can not do that (we have to implment
   softmmu again, just like rth said originally).

 - But lucky to me, Wine will manage the whole memory by its own, and
   also windows its own also manage its whole memory. They try to be as
   simple as they can. So I guess, current softmmu is enough to me.

> (2) If we do want to fix this, your fix is doing the wrong thing.
> It is correct that we don't mark the areas outside the guest page
> as PROT_VALID, because they are not valid guest memory. If you
> want to avoid the mmap() you need to change the condition we're using
> to decide whether to mmap() a fresh host page (it would need to
> look at the PROT_VALID bits within the new guest mapping, not just
> the ones outside it). Something like:
> 
> /* get the protection of the target pages outside the mapping,
>  * and check whether we already have a host page allocated
>  */
> prot1 = 0;
> havevalid = 0;
> for(addr = real_start; addr < real_end; addr++) {
> int pageprot = page_get_flags(addr);
> if (addr < start || addr >= end) {
> prot1 |= pageprot;
> }
> havevalid |= pageprot;
> }
> 
> if (!havevalid) {
> /* no page was there, so ... */
> ...
> }
>
 
What you said above sounds OK to me, if we don't consider about
MAP_PRIVATE or MAP_SHARED.

After think of again, for me: we need keep the current code no touch,
but the related comments "/* no page was there, so ... */" need be
improved, I guess, it should be:

 - If there is no host page or only one target page, we need call mmap2
   again, which will satisfy the parameter 'flags' (e.g. MAP_PRIVATE or
   MAP_SHARED), else FIXME: at present, the 'flags' has to be skipped.


> But I think it's only worth making this change if we're fixing
> a real bug where the guest behaves wrongly.
> 

It sounds OK to me.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()

2016-01-26 Thread Chen Gang

On 2016年01月26日 17:11, Peter Maydell wrote:
> On 26 January 2016 at 02:58, Chen Gang <cheng...@emindsoft.com.cn> wrote:
>> The related comments for  "if (prot1 == 0)" code block is "no page was
>> there, so we allocate one".
>>
>> So I guess this code block is not only allocate page for guest, but also
>> for host. So prot1 is not only for the guest page, but also for host
>> page.
> 
> The comment means specifically "allocate a host page".
> 

OK, thanks.

>> If we do not page_set_flags with PAGE_VALID, The next call
>> in mmap_frag for the same area will let prot1 be 0, so still
>> fall into "if (prot1 == 0)" code block.
> 
> But in what case will we call mmap_frag() again before we
> call page_set_flags() at the bottom of target_mmap()?
> That is what is not clear to me, and why I asked you to describe
> what the case is that you're seeing problems with.
> 

When I run WeChat.exe with i386 wine with qemu-i386 under sw_64 arch.

 - The related command:

   "./i386-linux-user/qemu-i386 -strace -L /upstream/i386_wine 
/upstream/i386_wine/usr/local/bin/wine "C:\\Program 
Files\\Tencent\\WeChat\\WeChat.exe" > ana/try/info-strace.log 2>&1"

 - The related output (no any munmap, 135168 = 128KB + 4KB):

   4600 
mmap2(0x0034,135168,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0) = 
0x0034
   4600 mmap2(0x0034,135168,PROT_READ,MAP_SHARED|MAP_FIXED,8,0) = 0x0034
   4600 rt_sigprocmask(SIG_SETMASK,0x0033f574,NULL) = 0
   4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f5d0) = 0
   4600 write(3,0x33f6cc,64) = 64
   4600 read(4,0x33f6cc,64) = 1
   4600 rt_sigprocmask(SIG_SETMASK,0x0033f5d0,NULL) = 0
   4600 close(8) = 0
   4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f674) = 0
   4600 mprotect(0x0016,65536,PROT_READ|PROT_WRITE) = 0
   4600 rt_sigprocmask(SIG_SETMASK,0x0033f674,NULL) = 0
   4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f990) = 0
   4600 
mmap2(0x0034,135168,PROT_NONE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED|MAP_NORESERVE,-1,0)
 = 0x0034

wine often does like above, map the same position multiple times.

> Reading the target_mmap() code, its intention seems to be:
>  (a) if the whole allocation fits in one host page, call
>  mmap_frag() once and then "goto the_end1"

Also yes to me.

>  (b) otherwise, we'll call mmap_frag() once for the start
>  of the guest mapping, and once for the end, which must
>  be two different host pages
> 

Also yes to me.

> So if you're seeing mmap_frag() called twice for the same
> host page then something is going wrong, but I'm not sure what.
> 

For the case I provide above, it can call mmap_frag() twice for the same
host page.

By the way, after have a full test again, all related issues are OK, it
seems we need not this patch to fix current issues, it is really very
strange to me!(maybe it is fixed by my other patches? I don't know)

At present, our sw_64 qemu-i386 status:

 - Can run notepad.exe correctly, can run acdsee5.0.exe setup program
   successfully.

 - The performance is acceptable, after optimize the wine code (simply
   use 32 split times instead of 2 for reserve_area recursion), the
   initialization speed is really quick enough. :-)

 - When run WeChat.exe, it can popup connection GUI box, but will quit
   under sw_64. But for x86_64 qemu-i386, it can run WeChat.exe
   correctly (although after enter main gui, it is not stable enough).


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()

2016-01-26 Thread Chen Gang


On 2016年01月26日 18:26, Peter Maydell wrote:
> On 26 January 2016 at 10:19, Chen Gang <cheng...@emindsoft.com.cn> wrote:
>> When I run WeChat.exe with i386 wine with qemu-i386 under sw_64 arch.
>>
>>  - The related command:
>>
>>"./i386-linux-user/qemu-i386 -strace -L /upstream/i386_wine 
>> /upstream/i386_wine/usr/local/bin/wine "C:\\Program 
>> Files\\Tencent\\WeChat\\WeChat.exe" > ana/try/info-strace.log 2>&1"
>>
>>  - The related output (no any munmap, 135168 = 128KB + 4KB):
>>
>>4600 
>> mmap2(0x0034,135168,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0) 
>> = 0x0034
>>4600 mmap2(0x0034,135168,PROT_READ,MAP_SHARED|MAP_FIXED,8,0) = 
>> 0x0034
>>4600 rt_sigprocmask(SIG_SETMASK,0x0033f574,NULL) = 0
>>4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f5d0) = 0
>>4600 write(3,0x33f6cc,64) = 64
>>4600 read(4,0x33f6cc,64) = 1
>>4600 rt_sigprocmask(SIG_SETMASK,0x0033f5d0,NULL) = 0
>>4600 close(8) = 0
>>4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f674) = 0
>>4600 mprotect(0x0016,65536,PROT_READ|PROT_WRITE) = 0
>>4600 rt_sigprocmask(SIG_SETMASK,0x0033f674,NULL) = 0
>>4600 rt_sigprocmask(SIG_BLOCK,0x7bced7e0,0x0033f990) = 0
>>4600 
>> mmap2(0x0034,135168,PROT_NONE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED|MAP_NORESERVE,-1,0)
>>  = 0x0034
>>
>> wine often does like above, map the same position multiple times.
> 
> That output seems to show all the mmap calls working fine, though.
>

OK, thanks.
 
>>
>> For the case I provide above, it can call mmap_frag() twice for the same
>> host page.
> 
> For the same single call to target_mmap() ? What is the code flow
> within QEMU that causes this?
> 

Within one single call to target_mmap(), it should be OK.

But multiple call to target_mmap(), may call mmap_frag() multiple times
for the same host page (also for the same target page). In our case:

 - 4600 
mmap2(0x0034,135168,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0) = 
0x0034

   It will call mmap_frag() with start address 0x0034 + 128KB, and
   set the target page with PAGE_VALID. But left the half below host
   page without PAGE_VALID.

 - 4600 mmap2(0x0034,135168,PROT_READ,MAP_SHARED|MAP_FIXED,8,0) = 0x0034

   It will call mmap_frag() with start address 0x0034 + 128KB, and
   check the half below host page which has no PAGE_VALID, then "prot1
   == 0", mmap_frag() thinks "no page was there, so we allocate one".

 - But in fact, the first mmap_frag() has already allocated one page at
   0x0034 + 128KB.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v2 1/3] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()

2016-01-25 Thread Chen Gang

Firstly, thank you very much to still focus on this patch, which will
be much helpful for my current work! 

And I modified some code, but did not send patches to upstream, e.g.
sw_64 related code, use TARGET_PAGE_SIZE instead of HOST_PAGE_SIZE ...).

  I skipped MAP_SHARED with PROT_WRITE check to skip some another issues,
  (since I guess for running wine, it should be OK). maybe it be related
  with this issue?

  diff --git a/linux-user/mmap.c b/linux-user/mmap.c
  index 86c270b..a76450a 100644
  --- a/linux-user/mmap.c
  +++ b/linux-user/mmap.c
  @@ -170,12 +170,13 @@ static int mmap_frag(abi_ulong real_start,
   
   prot_new = prot | prot1;
   if (!(flags & MAP_ANONYMOUS)) {
  +#if 0
   /* msync() won't work here, so we return an error if write is
  possible while it is a shared mapping */
   if ((flags & MAP_TYPE) == MAP_SHARED &&
   (prot & PROT_WRITE))
   return -1;
  -
  +#endif
   /* adjust protection to be able to read */
   if (!(prot1 & PROT_WRITE))
   mprotect(host_start, qemu_host_page_size, prot1 | PROT_WRITE);
  

And the other related reply for this thread is at the bottom of this
mail, please check.

On 2016年01月25日 23:07, Peter Maydell wrote:
> On 11 January 2016 at 09:01,  <cheng...@emindsoft.com.cn> wrote:
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> mmap() size in mmap_frag() is qemu_host_page_size, but the outside calls
>> page_set_flags() may be not with qemu_host_page_size. So after mmap(),
>> call page_set_flags() in time.
>>
>> After this fix,  for the next call for the same region, prot1 will be
>> PAGE_VALID (not 0), so can avoid to enter "if (prot1 == 0)" case, again.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/mmap.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 445e8c6..7807ed0 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -162,6 +162,8 @@ static int mmap_frag(abi_ulong real_start,
>> flags | MAP_ANONYMOUS, -1, 0);
>>  if (p == MAP_FAILED)
>>  return -1;
>> +page_set_flags(real_start, real_start + qemu_host_page_size,
>> +   PAGE_VALID);
>>  prot1 = prot;
>>  }
>>  prot1 &= PAGE_BITS;
> 
> I'm confused about this change, because as far as I can see
> page_set_flags/page_get_flags work on guest pages, not host pages.
> So setting the flags for the whole of the host page to PAGE_VALID
> doesn't seem right -- the other guest pages within this host page
> are not valid. And the VALID bit should be set for the guest page
> that is within the mapping as part of the call to page_set_flags()
> at the bottom of target_mmap().
> 

The related comments for  "if (prot1 == 0)" code block is "no page was
there, so we allocate one".

So I guess this code block is not only allocate page for guest, but also
for host. So prot1 is not only for the guest page, but also for host
page.

If we do not page_set_flags with PAGE_VALID, The next call in mmap_frag
for the same area will let prot1 be 0, so still fall into "if (prot1 ==
0)" code block.

> It would help if you could explain what the failure case is that
> you're trying to fix, and how the current code fails.
> 

I shall give a full test again, and provide the test result later.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let lv always match val in do_getsockopt()

2016-01-14 Thread Chen Gang

On 2016年01月14日 17:41, Laurent Vivier wrote:
> 
> Le 14/01/2016 10:37, Chen Gang a écrit :
>>
>> On 2016年01月14日 17:10, Laurent Vivier wrote:
>>>
>>> Le 14/01/2016 10:01, Chen Gang a écrit :
>>>>
>>>> I am not quite sure whether kernel always returns sizeof(int) (I guess,
>>>> it should be).
>>>
>>> it can be 1 only if len is 1, but this is managed below.
>>>
>>
>> Excuse me, I do not quite understand your meaning.
>>
>> For me, if we are sure lv is always 4 for SO_TYPE, we don't need this
>> patch.
>>
>> If lv may be 1 for SO_TYPE (it means val >> 8 may be zero), after call
>> host_to_target_sock_type, val >> 8 may be non-zero, theoretically. In
>> this case, we need modify lv to 4.
>>
>>>> For me, if you are sure, we can skip this patch.
>>>
>>> Does it fix something ?
>>>
>>
>> It is only theoretical, not real world, at present.
> 
> So I think we can skip it.
> 

It is OK to me.


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] translate-all.c: Remove writable protection feature for tb_alloc_page()

2016-01-14 Thread Chen Gang
On 2016年01月14日 18:30, Peter Maydell wrote:
> On 14 January 2016 at 10:26, Chen Gang <cheng...@emindsoft.com.cn> wrote:
>> On 2016年01月14日 18:05, Peter Maydell wrote:
>>> If we don't mark the page as non-writeable when we generate a TB
>>> from it, how do we detect when guest code later writes to that
>>> page (which means we need to invalidate the TB) ?
>>>
>>
>> For me, what you said above sounds reasonable, at present, that's really
>> valuable to me :-)
>>
>> I guess, you also mean: our qemu will catch the host page fault signal
>> and invalidate the TB.
> 
> Yes, this is how it works for user-mode. (For softmmu we can catch
> writes and send them via the slow path which does the check for
> whether TBs need to be invalidated; for linux-user we have no
> emulated MMU so we must rely on the host kernel sending us the
> SIGSEGV.) The bit of code that does this is at the top of
> handle_cpu_signal():
> 
> if (is_write && h2g_valid(address)
> && page_unprotect(h2g(address), pc, puc)) {
> return 1;
> }
> 

OK, thank you very much!  :-)


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let lv always match val in do_getsockopt()

2016-01-14 Thread Chen Gang

On 2016年01月14日 17:10, Laurent Vivier wrote:
> 
> Le 14/01/2016 10:01, Chen Gang a écrit :
>>
>> I am not quite sure whether kernel always returns sizeof(int) (I guess,
>> it should be).
> 
> it can be 1 only if len is 1, but this is managed below.
> 

Excuse me, I do not quite understand your meaning.

For me, if we are sure lv is always 4 for SO_TYPE, we don't need this
patch.

If lv may be 1 for SO_TYPE (it means val >> 8 may be zero), after call
host_to_target_sock_type, val >> 8 may be non-zero, theoretically. In
this case, we need modify lv to 4.

>> For me, if you are sure, we can skip this patch.
> 
> Does it fix something ?
> 

It is only theoretical, not real world, at present.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let lv always match val in do_getsockopt()

2016-01-14 Thread Chen Gang
On 2016年01月14日 16:15, Laurent Vivier wrote:
> Le 14/01/2016 07:24, cheng...@emindsoft.com.cn a écrit :
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> After host_to_target_sock_type(), the length of val may be changed, so
>> calculate the related lv, too.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/syscall.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index fcdca2a..0e95f35 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -1841,6 +1841,7 @@ static abi_long do_getsockopt(int sockfd, int level, 
>> int optname,
>>  return ret;
>>  if (optname == SO_TYPE) {
>>  val = host_to_target_sock_type(val);
>> +lv = (val >> 8) ? 4 : 1;
> 
> It seems the kernel always returns sizeof(int) (for all archs), what is
> the aim of reducing the size ?
> 

I am not quite sure whether kernel always returns sizeof(int) (I guess,
it should be).

For me, if you are sure, we can skip this patch.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] translate-all.c: Remove writable protection feature for tb_alloc_page()

2016-01-14 Thread Chen Gang
On 2016年01月14日 18:05, Peter Maydell wrote:
> On 14 January 2016 at 06:03,  <cheng...@emindsoft.com.cn> wrote:
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> Guest may allocate a readable, writable, and executable page, then write
>> data on the page, and execute data as code on the page too, then write
>> anther data still within the page.
>>
>> So remove this feature from linux-user: it not only consumes a little
>> performance, but also causes issue with the old Linux kernel under some
>> of architectures (they will directly generate segment fault for it).
> 
> If we don't mark the page as non-writeable when we generate a TB
> from it, how do we detect when guest code later writes to that
> page (which means we need to invalidate the TB) ?
> 

For me, what you said above sounds reasonable, at present, that's really
valuable to me :-)

I guess, you also mean: our qemu will catch the host page fault signal
and invalidate the TB.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v2] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt

2016-01-11 Thread Chen Gang
On 1/11/16 19:04, Peter Maydell wrote:
> On 11 January 2016 at 10:31, Chen Gang <cheng...@emindsoft.com.cn> wrote:
> [various things about a patch]
> 
> Why have we dropped qemu-devel from this email thread?
> 

Oh, sorry, I did not notice about it.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt

2016-01-10 Thread Chen Gang

Oh, sorry, after check again, I guess, we need continue to discuss.

On 2016年01月08日 17:40, Chen Gang wrote:
> 
> On 2016年01月08日 16:25, Laurent Vivier wrote:
>>
>>> +if (optlen < sizeof(struct target_timeval)) {
>>> +return -TARGET_EINVAL;
>>> +}
>>
>> You don't have to check the len (kernel doesn't), EINVAL is not listed
>> in the getsockopt() error cases, it should be an EFAULT, and this will
>> be managed by copy_to_user_timeval().
>>
 

After "man getsockopt", there is EINVAL in its error cases.

> OK.
> 
>>> +lv = sizeof(tv);
>>> +ret = get_errno(getsockopt(sockfd, level, optname, , ));
>>> +if (ret < 0) {
>>> +return ret;
>>> +}
>>
>>  if (len > lv)
>> len = lv;
>>

For me, len is for target, lv is for host, they cann't be compared with
each other.

> 
> OK.
> 
>>> +if (copy_to_user_timeval(optval_addr, )) {
>>> +return -TARGET_EFAULT;
>>> +}
>>> +if (put_user_u32(sizeof(struct target_timeval), optlen)) {
>>> +return -TARGET_EFAULT;
>>> +}
>>
>> if (put_user_u32(len, optlen))
>> return -TARGET_EFAULT;
>>

For me, we still need put sizeof(struct target_timeval) to optlen, since
the host lengh should be sizeof(struct timeval).

> 
> OK. I shall send patch v2 for it, in the next week.
> 
> 

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let tilegx support sigaltstack

2016-01-10 Thread Chen Gang

On 1/10/16 18:43, Laurent Vivier wrote:
> Michael Tokarev has already enable sigaltstack() for all architecture:
> 
> https://patchwork.ozlabs.org/patch/561514/
> 
> It is in the "-for-upstream" tree of Riku.
> 

OK, thanks. I shall continue solving existing issues for qemu tilegx,
during my free time.


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt

2016-01-08 Thread Chen Gang

Firstly, thank you very much for your careful work.

On 2016年01月08日 16:25, Laurent Vivier wrote:
> 
> 
> Le 08/01/2016 02:59, cheng...@emindsoft.com.cn a écrit :
[...]
>> @@ -1692,10 +1693,30 @@ static abi_long do_getsockopt(int sockfd, int level, 
>> int optname,
>>  switch (optname) {
>>  /* These don't just return a single integer */
>>  case TARGET_SO_LINGER:
>> -case TARGET_SO_RCVTIMEO:
>> -case TARGET_SO_SNDTIMEO:
>>  case TARGET_SO_PEERNAME:
>>  goto unimplemented;
>> +
> 
> useless blank line
> 

OK.

>> +case TARGET_SO_RCVTIMEO:
>> +optname = SO_RCVTIMEO;
>> +goto time_case;
>> +case TARGET_SO_SNDTIMEO:
>> +optname = SO_SNDTIMEO;
>> +time_case:
> 
> Something like in "int_case", I think optlen is a pointer, not the length:
> 
> if (get_user_u32(len, optlen))
> return -TARGET_EFAULT;
> if (len < 0)
> return -TARGET_EINVAL;
> 

OK.

> 
>> +if (optlen < sizeof(struct target_timeval)) {
>> +return -TARGET_EINVAL;
>> +}
> 
> You don't have to check the len (kernel doesn't), EINVAL is not listed
> in the getsockopt() error cases, it should be an EFAULT, and this will
> be managed by copy_to_user_timeval().
> 

OK.

>> +lv = sizeof(tv);
>> +ret = get_errno(getsockopt(sockfd, level, optname, , ));
>> +if (ret < 0) {
>> +return ret;
>> +}
> 
>  if (len > lv)
> len = lv;
> 

OK.

>> +if (copy_to_user_timeval(optval_addr, )) {
>> +return -TARGET_EFAULT;
>> +}
>> +if (put_user_u32(sizeof(struct target_timeval), optlen)) {
>> +return -TARGET_EFAULT;
>> +}
> 
> if (put_user_u32(len, optlen))
> return -TARGET_EFAULT;
> 

OK. I shall send patch v2 for it, in the next week.


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user: syscall: Add SO_LINGER for setsockopt

2016-01-08 Thread Chen Gang

On 2016年01月08日 17:57, Laurent Vivier wrote:
> 
>>>> +return get_errno(setsockopt(sockfd, SOL_SOCKET, optname,
>>>> + , sizeof(lg)));
>>>
>>> Why do you use "SOL_SOCKET" instead of "level" ?
>>>
>>
>> At present, level is TARGET_SOL_SOCKET, but we need SOL_SOCKET.
> 
> Yes, you're right... so there is a bug in TARGET_SO_BINDTODEVICE which
> is using "level" :)
> 

OK, thanks. I shall send patch v2 for it, next week.

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user: syscall: Add SO_LINGER for setsockopt

2016-01-08 Thread Chen Gang

On 2016年01月08日 16:38, Laurent Vivier wrote:
> 
>> +if (!lock_user_struct(VERIFY_READ, tlg, optval_addr, 1)) {
>> +return -TARGET_EFAULT;
>> +}
>> +__get_user(lg.l_onoff, >l_onoff);
>> +__get_user(lg.l_linger, >l_linger);
>> +unlock_user_struct(tlg, optval_addr, 0);
> 
> You can't unlock the structure you're going to use.
> 

OK, thanks.


>> +return get_errno(setsockopt(sockfd, SOL_SOCKET, optname,
>> + , sizeof(lg)));
> 
> Why do you use "SOL_SOCKET" instead of "level" ?
> 

At present, level is TARGET_SOL_SOCKET, but we need SOL_SOCKET.


>> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> index 9d3c537..5a4d565 100644
>> --- a/linux-user/syscall_defs.h
>> +++ b/linux-user/syscall_defs.h
>> @@ -165,6 +165,11 @@ struct target_ip_mreq_source {
>>  uint32_t imr_sourceaddr;
>>  };
>>  
>> +struct target_linger {
>> +int l_onoff;/* Linger active*/
>> +int l_linger;   /* How long to linger for   */
>> +};
>> +
> 
> Must be "abi_int" to force good alignment for the target.
> 

OK, thanks.


-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v2] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()

2016-01-07 Thread Chen Gang
On 2016年01月08日 03:35, Peter Maydell wrote:
> 
> Please don't do multiple things in a single patch. This patch
> has all of:
>  * a fix for an unnecessary inefficiency
>  * a coding style change with no functional effects
>  * a bug fix
> 
> Mixing them up together like this makes it harder to evaluate
> the bug fix, which is the important part of the change.
> 

OK, thanks.

>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 445e8c6..7920c5e 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -151,17 +151,19 @@ static int mmap_frag(abi_ulong real_start,
>>
>>  /* get the protection of the target pages outside the mapping */
>>  prot1 = 0;
>> -for(addr = real_start; addr < real_end; addr++) {
>> +for (addr = real_start; addr < real_end; addr += TARGET_PAGE_SIZE) {
>>  if (addr < start || addr >= end)
>>  prot1 |= page_get_flags(addr);
>>  }
>>
>>  if (prot1 == 0) {
>>  /* no page was there, so we allocate one */
>> -void *p = mmap(host_start, qemu_host_page_size, prot,
>> -   flags | MAP_ANONYMOUS, -1, 0);
>> -if (p == MAP_FAILED)
>> +if (mmap(host_start, qemu_host_page_size, prot, flags | 
>> MAP_ANONYMOUS,
>> + -1, 0) == MAP_FAILED) {
>>  return -1;
>> +}
>> +page_set_flags(real_start, real_start + qemu_host_page_size,
>> +   prot | PAGE_VALID);
> 
> I'm not convinced this is right -- it will mean that we set
> the page flags for every target page in this host page to
> be the same thing (the ORed together result we calculated).
> I don't think we want to update the page flags like that --
> if one target page was read-only and the other read-write then
> we need to make the whole host page read-write (since it's
> bigger and covers both target pages), but we don't want to
> incorrectly record that the first target-page is read-write.
> 

OK, thanks. It looks this patch is not quite precise. I guess, we need
use PAGE_VALID instead of "prot | PAGE_VALID" for page_set_flags() after
mmap() in mmap_frag().

So when the next call for the same location, prot1 will be PAGE_VALID (
now, it may be 0), then can protect to enter "if (prot1 == 0)", again.


> I don't really understand this mmap code, though -- that's just
> the result of looking at it for fifteen minutes or so.
> 

OK, I can understand. 

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v5 1/5] fpu: softfloat: Add normalize_roundpack_float32 function

2016-01-02 Thread Chen Gang

For sig == 0 case, the original implementation is incorrect (although it
passes gcc testsuite), it needs to consider about sign for float_zero.

The related fix diff for it is below. After patches v5 are finished
reviewing, I shall merge the fix diff below to patch v6, next.

Thanks.

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index dba8566..5ad8bb5 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -7098,19 +7098,15 @@ float32 normalize_roundpack_float32(flag sign, 
int_fast16_t exp, uint32_t sig,
 return packFloat32(sign, 0, sig);
 }
 
+if (!sig) {
+return float32_set_sign(float32_zero, sign);
+}
+
 if (sign) {
 if (sig & 0x7FFF) {
 return normalizeRoundAndPackFloat32(1, exp - 2, sig, status);
 }
-if (sig) {
-return packFloat32(1, exp, 0);
-} else {
-return float32_zero;
-}
-}
-
-if (!sig) {
-return float32_zero;
+return packFloat32(1, exp, 0);
 }
 
 scount = countLeadingZeros64(absa) - 40;

On 1/3/16 06:25, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang <gang.chen.5...@gmail.com>
> 
> It is based on (u)int32_to_float32 function to support float32 packing.
> 
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
>  fpu/softfloat.c | 55 
> +
>  include/fpu/softfloat.h |  8 +++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index f1170fe..dba8566 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -7080,6 +7080,61 @@ float64 uint32_to_float64(uint32_t a, float_status 
> *status)
>  return int64_to_float64(a, status);
>  }
>  
> +/*
> + * The mantissa contents the hide bit, e.g. exp: 0x9e with sig: 1 means 1.0f.
> + *
> + * It references from int32_to_float32() and uint32_to_float32()
> + */
> +float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t 
> sig,
> +float_status *status)
> +{
> +uint64_t absa = sig;
> +int8_t scount;
> +
> +if (exp >= 0xff) {
> +return packFloat32(sign, 0xFF, 0);
> +} else if (exp <= 0) {
> +shift32RightJamming(sig, 0 - exp, );
> +return packFloat32(sign, 0, sig);
> +}
> +
> +if (sign) {
> +if (sig & 0x7FFF) {
> +return normalizeRoundAndPackFloat32(1, exp - 2, sig, status);
> +}
> +if (sig) {
> +return packFloat32(1, exp, 0);
> +} else {
> +return float32_zero;
> +}
> +}
> +
> +if (!sig) {
> +return float32_zero;
> +}
> +
> +scount = countLeadingZeros64(absa) - 40;
> +if (scount >= 0) {
> +exp -= 7 + scount + 2;
> +if (exp <= 0) {
> +return packFloat32(0, 0, absa);
> +}
> +return packFloat32(0, exp, absa << scount);
> +}
> +
> +scount += 7;
> +exp -= scount + 2;
> +if (exp <= 0) {
> +return packFloat32(0, 0, absa);
> +}
> +if (scount < 0) {
> +shift64RightJamming(absa, 0 - scount, );
> +} else {
> +absa <<= scount;
> +}
> +return roundAndPackFloat32(0, exp, absa, status);
> +}
> +
>  uint32 float32_to_uint32(float32 a, float_status *status)
>  {
>  int64_t v;
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index ded34eb..4995a15 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -422,6 +422,14 @@ int float32_is_signaling_nan( float32 );
>  float32 float32_maybe_silence_nan( float32 );
>  float32 float32_scalbn(float32, int, float_status *status);
>  
> +/*
> + * The mantissa contents the hide bit, e.g. exp: 0x9e with sig: 1 means 1.0f.
> + *
> + * It references from int32_to_float32() and uint32_to_float32()
> + */
> +float32 normalize_roundpack_float32(flag sign, int_fast16_t exp, uint32_t 
> sig,
> +float_status *status);
> +
>  static inline float32 float32_abs(float32 a)
>  {
>  /* Note that abs does *not* handle NaN specially, nor does
> 

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Support shared memory mapping in mmap_frag()

2015-12-31 Thread Chen Gang

Oh, sorry, please skip this patch (or discussion), I have send a new fix
patch for the related issue, so let the old ACDSee.exe and the latest
WinRAR.exe setup programs run successfully under real WinXP system dlls.

The related root cause is not related with shared memory, so at least now,
we can skip this patch (or discussion).

Thanks.

On 2015年12月30日 09:35, Chen Gang wrote:
> Hello all:
> 
> This patch is only for a discussion, I guess this patch is valuable for
> i386 wine running Windows.
> 
> Theoretically, this patch is incorrect, we have to implement softmmu to
> support different host and target pages (e.g. host 8KB, target 4KB):
> 
>  - If host 8KB is mapped as PRIVATE | FIXED, and runs a while (trigger
>copy on write).
> 
>  - Then map its low 4KB as SHARED | FIXED.
> 
>  - It has to fail.
> 
> So this patch is only a temporary fix:
> 
>  - It looks Windows maps shared memory with 64KB alignments (at least,
>at present, it is true).
> 
>  - OS will manage the process address area: let most shared memory area
>nearby.
> 
>  - After this patch, we can run Windows Notepad.exe and ACDSee5.0 setup
>program successfully. Next, I will run wine with the true windows
>system dlls for a test (e.g. windows own kernel32.dll).
> 
> I guess, we can use a switch macro or a new input parameter to enable or
> disable the related code (if they are really valuable enough for some
> using cases).
> 
> Thanks.
> 
> On 2015年12月30日 09:11, cheng...@emindsoft.com.cn wrote:
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> It is a temporary fix for i386 target system running Windows.
>>
>> Also remove useless variable 'p'.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/mmap.c |   22 +++---
>>  1 files changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 445e8c6..07758d4 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -156,12 +156,28 @@ static int mmap_frag(abi_ulong real_start,
>>  prot1 |= page_get_flags(addr);
>>  }
>>  
>> +/*
>> + * It is a temporary fix. Normally, target system will let shared memory
>> + * aligned with 64KB, and allocate them near with each other, no any 
>> other
>> + * kinds of mapping regions nearby. e.g. Windows on i386.
>> + */
>> +if ((start == real_start) && (flags & MAP_SHARED)) {
>> +if (prot1) {
>> +munmap(host_start, qemu_host_page_size);
>> +}
>> +if (mmap(host_start, qemu_host_page_size, prot, flags, fd, offset)
>> +== MAP_FAILED) {
>> +return -1;
>> +}
>> +return 0;
>> +}
>> +
>>  if (prot1 == 0) {
>>  /* no page was there, so we allocate one */
>> -void *p = mmap(host_start, qemu_host_page_size, prot,
>> -   flags | MAP_ANONYMOUS, -1, 0);
>> -if (p == MAP_FAILED)
>> +if (mmap(host_start, qemu_host_page_size, prot, flags | 
>> MAP_ANONYMOUS,
>> + -1, 0) == MAP_FAILED) {
>>  return -1;
>> +}
>>  prot1 = prot;
>>  }
>>  prot1 &= PAGE_BITS;
>>
> 

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Set prot page flags for the correct region in mmap_frag()

2015-12-31 Thread Chen Gang

Oh, sorry, for the coding style issues. Also the typo in the comments. I
shall send patch v2, next.

This fix patch can let the old ACDSee.exe, and the latest WinRAR.exe
setup programs run successfully under real WinXP system dlls.

Thanks.

On 2015年12月31日 15:14, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang <cheng...@emindsoft.com.cn>
> 
> mmap() size in mmap_frag() is qemu_host_page_size, but the outside calls
> page_set_flags() may be not with qemu_host_page_size. So after mmap(),
> call page_set_flags() in time.
> 
> Also let addr increasing step be TARGET_PAGE_SIZE, just like another
> areas have done.
> 
> Also remote redundant varialbe p.
> 
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
>  linux-user/mmap.c |   10 ++
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 445e8c6..7920c5e 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -151,17 +151,19 @@ static int mmap_frag(abi_ulong real_start,
>  
>  /* get the protection of the target pages outside the mapping */
>  prot1 = 0;
> -for(addr = real_start; addr < real_end; addr++) {
> +for(addr = real_start; addr < real_end; addr += TARGET_PAGE_SIZE) {
>  if (addr < start || addr >= end)
>  prot1 |= page_get_flags(addr);
>  }
>  
>  if (prot1 == 0) {
>  /* no page was there, so we allocate one */
> -void *p = mmap(host_start, qemu_host_page_size, prot,
> -   flags | MAP_ANONYMOUS, -1, 0);
> -if (p == MAP_FAILED)
> +if (mmap(host_start, qemu_host_page_size, prot, flags | 
> MAP_ANONYMOUS,
> + -1, 0) == MAP_FAILED) {
>  return -1;
> +}
> +page_set_flags(real_start, real_start + qemu_host_page_size,
> +   prot | PAGE_VALID);
>  prot1 = prot;
>  }
>  prot1 &= PAGE_BITS;
> 

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v3] linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()

2015-12-31 Thread Chen Gang

On 12/30/15 09:14, Laurent Vivier wrote:
> 
> Le 30/12/2015 02:10, cheng...@emindsoft.com.cn a écrit :
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> When mapping MAP_ANONYMOUS memory fragments, still need notice about to
>> set it zero, or it will cause issues.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/mmap.c |4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 7b459d5..c6c478e 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -186,10 +186,12 @@ static int mmap_frag(abi_ulong real_start,
>>  if (prot_new != (prot1 | PROT_WRITE))
>>  mprotect(host_start, qemu_host_page_size, prot_new);
>>  } else {
>> -/* just update the protection */
>>  if (prot_new != prot1) {
>>  mprotect(host_start, qemu_host_page_size, prot_new);
>>  }
>> +if (prot_new & PROT_WRITE) {
>> +memset(g2h(start), 0, end - start);
>> +}
>>  }
>>  return 0;
>>  }
>>
> 
> Reviewed-by: Laurent Vivier <laur...@vivier.eu>
> 

Thank you for your work.


Happy New Year!  :-)

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Support shared memory mapping in mmap_frag()

2015-12-29 Thread Chen Gang
Hello all:

This patch is only for a discussion, I guess this patch is valuable for
i386 wine running Windows.

Theoretically, this patch is incorrect, we have to implement softmmu to
support different host and target pages (e.g. host 8KB, target 4KB):

 - If host 8KB is mapped as PRIVATE | FIXED, and runs a while (trigger
   copy on write).

 - Then map its low 4KB as SHARED | FIXED.

 - It has to fail.

So this patch is only a temporary fix:

 - It looks Windows maps shared memory with 64KB alignments (at least,
   at present, it is true).

 - OS will manage the process address area: let most shared memory area
   nearby.

 - After this patch, we can run Windows Notepad.exe and ACDSee5.0 setup
   program successfully. Next, I will run wine with the true windows
   system dlls for a test (e.g. windows own kernel32.dll).

I guess, we can use a switch macro or a new input parameter to enable or
disable the related code (if they are really valuable enough for some
using cases).

Thanks.

On 2015年12月30日 09:11, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang <cheng...@emindsoft.com.cn>
> 
> It is a temporary fix for i386 target system running Windows.
> 
> Also remove useless variable 'p'.
> 
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
>  linux-user/mmap.c |   22 +++---
>  1 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 445e8c6..07758d4 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -156,12 +156,28 @@ static int mmap_frag(abi_ulong real_start,
>  prot1 |= page_get_flags(addr);
>  }
>  
> +/*
> + * It is a temporary fix. Normally, target system will let shared memory
> + * aligned with 64KB, and allocate them near with each other, no any 
> other
> + * kinds of mapping regions nearby. e.g. Windows on i386.
> + */
> +if ((start == real_start) && (flags & MAP_SHARED)) {
> +if (prot1) {
> +munmap(host_start, qemu_host_page_size);
> +}
> +if (mmap(host_start, qemu_host_page_size, prot, flags, fd, offset)
> +== MAP_FAILED) {
> +return -1;
> +}
> +return 0;
> +}
> +
>  if (prot1 == 0) {
>  /* no page was there, so we allocate one */
> -void *p = mmap(host_start, qemu_host_page_size, prot,
> -   flags | MAP_ANONYMOUS, -1, 0);
> -if (p == MAP_FAILED)
> +if (mmap(host_start, qemu_host_page_size, prot, flags | 
> MAP_ANONYMOUS,
> +     -1, 0) == MAP_FAILED) {
>  return -1;
> +}
>  prot1 = prot;
>  }
>  prot1 &= PAGE_BITS;
> 

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v4 2/4] target-tilegx: Add single floating point implementation

2015-12-27 Thread Chen Gang

On 12/24/15 23:52, Chen Gang wrote:
> On 12/24/15 07:07, Richard Henderson wrote:
> 
>> Moreover, I thought we agreed to do away with that CALC bit.
>>

After check again, I guess, we can stil reserve CALC bit:

 - Then we can remove float32_to_sfmt (use high 32-bit to save float32
   directly). And in helper_fsingle_pack2, for CALC, we can return high
   32-bit directly.  Only for NCALC, we need process it in details.

 - I guess, most cases are for CALC, so it will let the performance a
   little better (need float32_to_sfmt, then sfmt_to_float32 again).

Then we can only focus on NCAL in helper_fsingle_pack2.

Thanks.

> 
> OK, I will try, next.
> 
> I will copy and reconstruct related code from qemu fpu implementation
> instead of (u)int32/64_to_float32/64 functions (just like you said, I
> guess).
> 
> Hope I can finish within 2015-12-31.
> 
> Thanks.
> 

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v4 1/4] target-tilegx: Add floating point shared functions

2015-12-25 Thread Chen Gang

On 12/25/15 04:01, Richard Henderson wrote:
> On 12/24/2015 07:38 AM, Chen Gang wrote:
>>
>> OK, thanks. Since fp_status need to be initialized to be 0, so I will
>> declared it statically, too (need we consider about thread safe for it?
>> I guess not).
> 
> While qemu is not currently thread-safe, there's work going on to make that 
> happen.  There is no need to exacerbate the problem.
> 

OK, thanks.

> Also, I think using an on-stack automatic variable, initialized each time, 
> emphasizes the fact there there is no state that is preserved across 
> operations.
> 
> This should really be as simple as
> 
>   float_status fp_status = {
> .float_rounding_mode = float_round_nearest_even
>   };
> 
> (I realize float_round_nearest_even is *also* zero, but humor me.  At least 
> the other members are either flags or booleans.)
> 

OK, thanks.

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Use end instead of real_end in target_mmap

2015-12-24 Thread Chen Gang

On 12/24/15 17:54, Laurent Vivier wrote:
> 
> Le 24/12/2015 02:07, cheng...@emindsoft.com.cn a écrit :
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> In this case, real_end is larger than end, which may cause mmap_frag
>> process the incorrect memory region.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/mmap.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 7b459d5..57b0361 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -536,7 +536,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
>> prot,
>>  /* handle the end of the mapping */
>>  if (end < real_end) {
>>  ret = mmap_frag(real_end - qemu_host_page_size,
>> -real_end - qemu_host_page_size, real_end,
>> +real_end - qemu_host_page_size, end,
>>  prot, flags, fd,
>>  offset + real_end - qemu_host_page_size - 
>> start);
>>  if (ret == -1)
>>
> 
> The fragment must effectively be mapped only to "end" not to "real_end"
> (which is a host page aligned address, and thus this is not a fragment).
> It is consistent with what it is done in the case of one single page.
> 
> Reviewed-by: Laurent Vivier <laur...@vivier.eu>

Thank you for your comments. I only met this issue, and knew it should
be fixed in this way, but really don't know the related details.

Thanks.
-- 
Chen Gang

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



Re: [Qemu-devel] [PATCH v4 1/4] target-tilegx: Add floating point shared functions

2015-12-24 Thread Chen Gang

On 12/24/15 06:51, Richard Henderson wrote:
> On 12/23/2015 01:48 PM, cheng...@emindsoft.com.cn wrote:
>> +extern float_status fp_status;
> 
> No.  Locally declared in e.g. main_calc.
> 

OK, thanks. Since fp_status need to be initialized to be 0, so I will
declared it statically, too (need we consider about thread safe for it?
I guess not).


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v4 2/4] target-tilegx: Add single floating point implementation

2015-12-24 Thread Chen Gang
On 12/24/15 07:07, Richard Henderson wrote:
> On 12/23/2015 01:48 PM, cheng...@emindsoft.com.cn wrote:
>> +static float32 sfmt_to_float32(uint64_t sfmt)
>> +{
>> +uint32_t sign = get_fsingle_sign(sfmt);
>> +uint32_t man = get_fsingle_man(sfmt);
>> +uint32_t exp = get_fsingle_exp(sfmt);
>> +float32 f;
>> +
>> +if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) {
>> +if (sign) {
>> +f = int32_to_float32(0 - man, _status);
>> +} else {
>> +f = uint32_to_float32(man, _status);
>> +}
>> +exp += get_f32_exp(f) - 0x9e;
>> +if ((int32_t) exp < 0) {
>> +return float32_infinity | float32_set_sign(float32_zero, sign);
>> +} else if (exp >= 0xff) {
>> +return float32_zero | float32_set_sign(float32_zero, sign);
>> +} else {
>> +set_f32_exp(, exp);
>> +}
> 
> 
> What in the world are you attempting to do here?
> This is not normalization.  This is not even remotely
> correct with respect to zero or infinity.
> 

For fdouble, I use almost the same way, but can get the correct result (
pass gcc testsuite for fdouble, include inf and zero tests). But tests
will never enough, we can not say the fdouble implementation must be OK.

So please help check the fdouble implementation in details, again, when
you have time, it may still have issues.

> Moreover, I thought we agreed to do away with that CALC bit.
> 

OK, I will try, next.

I will copy and reconstruct related code from qemu fpu implementation
instead of (u)int32/64_to_float32/64 functions (just like you said, I
guess).

Hope I can finish within 2015-12-31.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/mmap.c: Use end instead of real_end in target_mmap

2015-12-23 Thread Chen Gang
Hello all:

After this patch, the i386 wine notepad.exe can be bootup and run under
sw_64 host.

 - The initialization is very very slow (about 10 minutes), it is mainly
   running in find_vma_reserved (consume more than 95% time resource). I
   guess, it can be optimized.

 - After initialization, for real using, the performance is acceptable!!
   :-)

 - Next, I shall try the real Windows XP notepad.exe.

I guess, we need not softmmu, at least, for wine, it is true (wine will
mmap all memory areas during initializing, and manage them by itself
when real running).

Welcome any suggestions, ideas, and completions.


BTW: Merry Christmas!  :-)

Thanks.

On 2015年12月24日 09:07, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang <cheng...@emindsoft.com.cn>
> 
> In this case, real_end is larger than end, which may cause mmap_frag
> process the incorrect memory region.
> 
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
>  linux-user/mmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 7b459d5..57b0361 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -536,7 +536,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  /* handle the end of the mapping */
>  if (end < real_end) {
>  ret = mmap_frag(real_end - qemu_host_page_size,
> -real_end - qemu_host_page_size, real_end,
> +real_end - qemu_host_page_size, end,
>  prot, flags, fd,
>  offset + real_end - qemu_host_page_size - start);
>  if (ret == -1)
> 

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation

2015-12-22 Thread Chen Gang

On 12/11/15 06:15, Chen Gang wrote:
> 
> On 12/11/15 04:18, Richard Henderson wrote:
>> On 12/10/2015 09:15 AM, Richard Henderson wrote:
>>>   d = (uint64_t)sign << 63;
>>>   d = deposit64(d, 53, 11, exp);
>>>   d = deposit64(d, 21, 32, man);
>>>   return float64_to_float32(d, fp_status);
>>
>> Hmm.  Actually, this incorrectly adds the implicit bit.  We'd actually need 
>> to
>> steal portions of softfloat.c to do this properly.  Which still isn't that
>> difficult.
>>

Oh, sorry, I misunderstood this reply.

> 
> Yes, thanks.
> 

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation

2015-12-21 Thread Chen Gang

On 12/21/15 23:01, Richard Henderson wrote:
> On 12/20/2015 07:30 AM, Chen Gang wrote:
>> And we have to still check TILEGX_F_CALC_CVT, for they are really two
>> different format: TILEGX_F_CALC_CVT has no HBIT, but TILEGX_F_CALC_NCVT
>> has HBIT (which we need process it specially).
> 
> The both do, in that you re-normalize to produce that HBIT.
> That's the whole point.
> 

Oh, yes.

But all together, we want to normalize the float value in fsingle_pack2,
so we can not use float64_to_float32(), it assumes the input is already
normalized (if we can let the input normalized, we will return directly).

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v2] linux-user/mmap.c: Always zero MAP_ANONYMOUS memory inmmap_frag()

2015-12-21 Thread Chen Gang

> From:  "Laurent Vivier";<laur...@vivier.eu>;
> 
> Le 21/12/2015 03:33, cheng...@emindsoft.com.cn a écrit :
>> From: Chen Gang <cheng...@emindsoft.com.cn>
>>
>> When mapping MAP_ANONYMOUS memory fragments, still need notice about to
>> set it zero, or it will cause issues.
> 
> Perhaps you can explain in the commit message why this page is not
> already filled by zeros ?
> 

In fact, I don't know. when I debug related issues under sw_64 host (
almost the same as alpha) for i386 target, I found it. The host is 8KB
page, but the guest is 4KB page. So I guess:

 - Firstly, qemu allocate one !MAP_ANONYMOUS 8KB page, so it is not
   zeroed.

 - Then qemu want one MAP_ANONYMOUS fragment, just can use the the left
   room of the page above. It merges and sets the related prot, but
   forgot to reset zero for MAP_ANONYMOUS fragment.

 - For normal host 4KB and guest also 4KB page, it is rarely happen (
   may never happen, I guess). But for host 8KB and guest 4KB pages, it
   often occurs.

Our qemu, at present, let i386 use 8KB page by force, but it can not
work. Our qemu members told me to use softmmu, or we would meet many
various issues.  But at present, I do not meet softmmu related issues:

 - I can chroot with binfmt_misc successfully by using sw_64 host i386
   static qemu, and most i386 programs (e.g. gcc, vi, xclac...) are OK
   (I build i386 wine under sw_64 host i386 chroot environments).

 - After fix this issue, some wine programs can run successfully, e.g.
   cmd.exe, clock.exe, winver.exe (they almost like windows own exe),
   but initialization is very slow in qemu mmap_find_vma_reserved().

 - Now, I am just analyzing and fixing the issue about notepad.exe, is
   it related with softmmu? I am not sure.

When I really meet softmmu related issue, I have to solve it, although
it may spend much time resources.


>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/mmap.c |4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 7b459d5..29fe646 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -186,10 +186,12 @@ static int mmap_frag(abi_ulong real_start,
>>  if (prot_new != (prot1 | PROT_WRITE))
>>  mprotect(host_start, qemu_host_page_size, prot_new);
>>  } else {
>> -/* just update the protection */
>>  if (prot_new != prot1) {
>>  mprotect(host_start, qemu_host_page_size, prot_new);
>>  }
>> +if ((prot_new & PROT_WRITE) && ((flags & MAP_PRIVATE) || (fd == 
>> -1))) {
> 
> According to manpage, for MAP_ANONYMOUS, fd can be ignored.
> Why do you check if the page is MAP_PRIVATE or not ?
> 

For me, we can remove them, originally I only worry about MAP_SHARED and
fd according to the man page (it mentions MAP_SHARED and fd):

  MAP_ANONYMOUS
The mapping is not backed by any file; its contents are initialized
to zero. The fd and offset arguments are ignored; however, some
implementations require fd to be -1  if MAP_ANONYMOUS (or MAP_ANON)
is specified, and portable applications should ensure this.  The use
of MAP_ANONYMOUS in conjunction with MAP_SHARED is supported on
Linux only since kernel 2.4.


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

2015-12-20 Thread Chen Gang

On 2015年12月18日 18:47, Chen Gang wrote:
> 
> On 2015年12月18日 17:41, Laurent Vivier wrote:
>>
>> Le 18/12/2015 07:51, Chen Gang a écrit :

[...]

>>>
>>> +++ b/linux-user/mmap.c
>>> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
>>> int prot,
>>>  printf("\n");
>>>  #endif
>>>  tb_invalidate_phys_range(start, start + len);
>>> +if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
>>> +&& ((flags & MAP_PRIVATE) || (fd == -1))) {
>>> +memset(g2h(start), 0, len);
>>> +}
>>
>> IMHO, their kernel needs a fix, mmap(2):
>>
>> MAP_ANONYMOUS
>> The mapping is not backed by any file; its contents are initial‐
>> ized to zero.
>>

Oh, sorry, after check again, it is not kernel's issue: mmap() will
always zero the ANONYMOUS memory for kernel 3.8 in sw_64 arch.

It is still our qemu's issue in mmap_frag (do not zero ANONYMOUS memory
fragments), so I sent patch v2 for it, please help check.

Thanks.

> 
> OK, Thanks.
> 
> 

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation

2015-12-20 Thread Chen Gang

After tried, I guess, this way below is incorrect: float64_to_float32()
assumes the input 'd' is already a standard (packed) float64 variable.
But in fact, it is not (e.g. the input from floatsisf2).

And we have to still check TILEGX_F_CALC_CVT, for they are really two
different format: TILEGX_F_CALC_CVT has no HBIT, but TILEGX_F_CALC_NCVT
has HBIT (which we need process it specially).

For me, the way like helper_fdouble_pack2 (the double implementation) is
OK to TILEGX_F_CALC_NCVT format, too.

 - Shift left to get HBIT, and change the related vexp (use vexp instead
   of exp to process overflow cases -- like double implementation does).

 - Use (u)int32_to_float32 for the mantissa.

 - Then process exp again.


Thanks.

On 12/11/15 06:14, Chen Gang wrote:
>> In particular, if gcc decided to optimize fractional fixed-point types, it
>> > would do something very similar to the current floatsisf2 code sequence, 
>> > except
>> > that it wouldn't use 0x9e as the exponent; it would use something smaller, 
>> > so
>> > that some number of low bits of the mantessa would be below the radix 
>> > point.
>> > 
> Oh, really.
> 
>> > Therefore, I think that fsingle_pack2 should do the following: Take the
>> > (sign,exp,man) tuple and slot them into a double -- recall that a single 
>> > only
>> > has 23 bits in its mantessa, and this temp format has 32 -- then convert 
>> > the
>> > double to a single.  Pre-rounded single results from fsingle_* will be
>> > unchanged, while integer data that gcc has constructed will be properly 
>> > rounded.
>> > 
>> > E.g.
>> > 
>> >   uint32_t sign = get_fsingle_sign(sfmt);
>> >   uint32_t exp = get_fsingle_exp(sfmt);
>> >   uint32_t man = get_fsingle_man(sfmt);
>> >   uint64_t d;
>> > 
>> >   /* Adjust the exponent for double precision, preserving Inf/NaN.  */
>> >   if (exp == 0xff) {
>> > exp = 0x7ff;
>> >   } else {
>> > exp += 1023 - 127;
>> >   }
>> > 
>> >   d = (uint64_t)sign << 63;
>> >   d = deposit64(d, 53, 11, exp);
>> >   d = deposit64(d, 21, 32, man);
>> >   return float64_to_float32(d, fp_status);
>> > 
>> > Note that this does require float32_to_sfmt to store the mantissa
>> > left-justified. That is, not in bits [54-32] as you're doing now, but in 
>> > bits
>> > [63-41].
>> > 
> For me, it is a good idea! :-)
> 
> 

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

2015-12-18 Thread Chen Gang

On 2015年12月18日 17:41, Laurent Vivier wrote:
> 
> 
> Le 18/12/2015 07:51, Chen Gang a écrit :
>>
>> I found this issue during my working time, it is about sw_64 (almost the
>> same as alpha) host running i386 wine programs.
>>
>> I also found another issue, but I am not quite sure whether it is worth
>> enough for our upstream: The related fix patch is below, which will let
>> the initialization slower, but for most archs, they have no this issue.
>>
>> linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap()
>> 
>> In some architectures, they have no policy to zero MAP_ANONYMOUS memory,
>>     which will cause issue for qemu target_mmap.
>> 
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 7b459d5..9c9152d 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
>> int prot,
>>  printf("\n");
>>  #endif
>>  tb_invalidate_phys_range(start, start + len);
>> +if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
>> +&& ((flags & MAP_PRIVATE) || (fd == -1))) {
>> +memset(g2h(start), 0, len);
>> +}
> 
> IMHO, their kernel needs a fix, mmap(2):
> 
> MAP_ANONYMOUS
> The mapping is not backed by any file; its contents are initial‐
> ized to zero.
> 

OK, Thanks.


-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

2015-12-18 Thread Chen Gang

On 12/18/15 17:37, Laurent Vivier wrote:
> 
> Le 18/12/2015 07:26, Chen Gang a écrit :
>>
>> For fcntl, it always needs to notice about it, just like do_fcntl() has
>> done, or it will cause issue (e.g. alpha host run i386 guest).
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  linux-user/syscall.c |   18 --
>>  1 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 0f8adeb..1a60e6f 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>> arg1,
>>  if (((CPUARMState *)cpu_env)->eabi) {
>>  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>>  goto efault;
>> -fl.l_type = tswap16(target_efl->l_type);
>> +fl.l_type = 
>> target_to_host_bitmask(tswap16(target_fl->l_type),
>> +   flock_tbl);
>>  fl.l_whence = tswap16(target_efl->l_whence);
>>  fl.l_start = tswap64(target_efl->l_start);
>>  fl.l_len = tswap64(target_efl->l_len);
>> @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>> arg1,
>>  {
>>  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>>  goto efault;
>> -fl.l_type = tswap16(target_fl->l_type);
>> +fl.l_type = 
>> target_to_host_bitmask(tswap16(target_fl->l_type),
>> +   flock_tbl);
>>  fl.l_whence = tswap16(target_fl->l_whence);
>>  fl.l_start = tswap64(target_fl->l_start);
>>  fl.l_len = tswap64(target_fl->l_len);
>> @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>> arg1,
>>  if (((CPUARMState *)cpu_env)->eabi) {
>>  if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 
>> 0)) 
>>  goto efault;
>> -target_efl->l_type = tswap16(fl.l_type);
>> +target_efl->l_type = host_to_target_bitmask(
>> + tswap16(fl.l_type), 
>> flock_tbl);
>>  target_efl->l_whence = tswap16(fl.l_whence);
>>  target_efl->l_start = tswap64(fl.l_start);
>>  target_efl->l_len = tswap64(fl.l_len);
>> @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>> arg1,
>>  {
>>  if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 
>> 0)) 
>>  goto efault;
>> -target_fl->l_type = tswap16(fl.l_type);
>> +target_fl->l_type = host_to_target_bitmask(
>> + tswap16(fl.l_type), 
>> flock_tbl);
>>  target_fl->l_whence = tswap16(fl.l_whence);
>>  target_fl->l_start = tswap64(fl.l_start);
>>  target_fl->l_len = tswap64(fl.l_len);
>> @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>> arg1,
>>  if (((CPUARMState *)cpu_env)->eabi) {
>>  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>>  goto efault;
>> -fl.l_type = tswap16(target_efl->l_type);
>> +fl.l_type = 
>> target_to_host_bitmask(tswap16(target_fl->l_type),
>> +   flock_tbl);
>>  fl.l_whence = tswap16(target_efl->l_whence);
>>  fl.l_start = tswap64(target_efl->l_start);
>>  fl.l_len = tswap64(target_efl->l_len);
>> @@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
>> arg1,
>>  {
>>  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>>  goto efault;
>> -fl.l_type = tswap16(target_fl->l_type);
>> +fl.l_type = 
>> target_to_host_bitmask(tswap16(target_fl->l_type),
>> +   flock_tbl);
>>  fl.l_whence = tswap16(target_fl->l_whence);
>>  fl.l_start 

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

2015-12-18 Thread Chen Gang

On 12/19/15 06:15, Laurent Vivier wrote:
> 
> Le 18/12/2015 23:12, Chen Gang a écrit :
>>

[...]

>>
>> OK, thank you very much. I shall config my email client again to notice
>> about it.
> 
> You should not use your email client to send patches, you should use
> "git send-email":
> 
> http://wiki.qemu.org/Contribute/SubmitAPatch
> 

OK, thanks.

-- 
Chen Gang

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



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

2015-12-18 Thread Chen Gang

On 12/19/15 05:58, Laurent Vivier wrote:
> 
> Le 18/12/2015 22:40, Chen Gang a écrit :
>>

[...]

>> I did not get any script/checkpatch.pl complains, originally.
>>
>> Does my email client configuration is incorrect, then cause incorrect
>> mail format? I guess not. The related reason is below.
>>
>>  - I copy your full reply mail contents to a new file (diff.patch).
> 
> If you copy, you loose the special characters. I do a "Save as".
> 
>>
>>  - Remove all '> ' in vi editor (1,% s/^> //g) (so get the original
>>patch contents).
>>
>>  - ./script/checkpatch.pl diff.patch, it has no any complains.
> 
> If I run "file" on the saved file, I have:
> 
> $ file orig.eml
> orig.eml: SMTP mail, ASCII text, with CRLF line terminators
> 
> I can convert it with "tr":
> 
> $ tr "\r" "\n" < orig.eml > new.eml
> 
> $ file new.eml
> new.eml: SMTP mail, ASCII text
> 
> $ ./scripts/checkpatch.pl new.eml
> total: 0 errors, 0 warnings, 54 lines checked
> 
> new.eml has no obvious style problems and is ready for submission.
> 
> Laurent
> 

OK, thank you very much. I shall config my email client again to notice
about it.


Thanks.
-- 
Chen Gang

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



[Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

2015-12-17 Thread Chen Gang

For fcntl, it always needs to notice about it, just like do_fcntl() has
done, or it will cause issue (e.g. alpha host run i386 guest).

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 linux-user/syscall.c |   18 --
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 0f8adeb..1a60e6f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 if (((CPUARMState *)cpu_env)->eabi) {
 if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
 goto efault;
-fl.l_type = tswap16(target_efl->l_type);
+fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
+   flock_tbl);
 fl.l_whence = tswap16(target_efl->l_whence);
 fl.l_start = tswap64(target_efl->l_start);
 fl.l_len = tswap64(target_efl->l_len);
@@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 {
 if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
 goto efault;
-fl.l_type = tswap16(target_fl->l_type);
+fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
+   flock_tbl);
 fl.l_whence = tswap16(target_fl->l_whence);
 fl.l_start = tswap64(target_fl->l_start);
 fl.l_len = tswap64(target_fl->l_len);
@@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 if (((CPUARMState *)cpu_env)->eabi) {
 if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) 
 goto efault;
-target_efl->l_type = tswap16(fl.l_type);
+target_efl->l_type = host_to_target_bitmask(
+ tswap16(fl.l_type), 
flock_tbl);
 target_efl->l_whence = tswap16(fl.l_whence);
 target_efl->l_start = tswap64(fl.l_start);
 target_efl->l_len = tswap64(fl.l_len);
@@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 {
 if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
 goto efault;
-target_fl->l_type = tswap16(fl.l_type);
+target_fl->l_type = host_to_target_bitmask(
+ tswap16(fl.l_type), 
flock_tbl);
 target_fl->l_whence = tswap16(fl.l_whence);
 target_fl->l_start = tswap64(fl.l_start);
 target_fl->l_len = tswap64(fl.l_len);
@@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 if (((CPUARMState *)cpu_env)->eabi) {
 if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
 goto efault;
-fl.l_type = tswap16(target_efl->l_type);
+fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
+   flock_tbl);
 fl.l_whence = tswap16(target_efl->l_whence);
 fl.l_start = tswap64(target_efl->l_start);
 fl.l_len = tswap64(target_efl->l_len);
@@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 {
 if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
 goto efault;
-fl.l_type = tswap16(target_fl->l_type);
+fl.l_type = target_to_host_bitmask(tswap16(target_fl->l_type),
+   flock_tbl);
 fl.l_whence = tswap16(target_fl->l_whence);
 fl.l_start = tswap64(target_fl->l_start);
 fl.l_len = tswap64(target_fl->l_len);
-- 
1.7.3.4



Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

2015-12-17 Thread Chen Gang

I found this issue during my working time, it is about sw_64 (almost the
same as alpha) host running i386 wine programs.

I also found another issue, but I am not quite sure whether it is worth
enough for our upstream: The related fix patch is below, which will let
the initialization slower, but for most archs, they have no this issue.

linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap()

In some architectures, they have no policy to zero MAP_ANONYMOUS memory,
which will cause issue for qemu target_mmap.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 7b459d5..9c9152d 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
prot,
 printf("\n");
 #endif
 tb_invalidate_phys_range(start, start + len);
+if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
+&& ((flags & MAP_PRIVATE) || (fd == -1))) {
+memset(g2h(start), 0, len);
+}
 mmap_unlock();
 return start;
 fail:


Thanks.

On 2015年12月18日 14:26, Chen Gang wrote:
> 
> For fcntl, it always needs to notice about it, just like do_fcntl() has
> done, or it will cause issue (e.g. alpha host run i386 guest).
> 
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
>  linux-user/syscall.c |   18 --
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 0f8adeb..1a60e6f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  if (((CPUARMState *)cpu_env)->eabi) {
>  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>  goto efault;
> -fl.l_type = tswap16(target_efl->l_type);
> +fl.l_type = 
> target_to_host_bitmask(tswap16(target_fl->l_type),
> +   flock_tbl);
>  fl.l_whence = tswap16(target_efl->l_whence);
>  fl.l_start = tswap64(target_efl->l_start);
>  fl.l_len = tswap64(target_efl->l_len);
> @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  {
>  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>  goto efault;
> -fl.l_type = tswap16(target_fl->l_type);
> +fl.l_type = 
> target_to_host_bitmask(tswap16(target_fl->l_type),
> +   flock_tbl);
>  fl.l_whence = tswap16(target_fl->l_whence);
>  fl.l_start = tswap64(target_fl->l_start);
>  fl.l_len = tswap64(target_fl->l_len);
> @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  if (((CPUARMState *)cpu_env)->eabi) {
>  if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 
> 0)) 
>  goto efault;
> -target_efl->l_type = tswap16(fl.l_type);
> +target_efl->l_type = host_to_target_bitmask(
> + tswap16(fl.l_type), 
> flock_tbl);
>  target_efl->l_whence = tswap16(fl.l_whence);
>  target_efl->l_start = tswap64(fl.l_start);
>  target_efl->l_len = tswap64(fl.l_len);
> @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  {
>  if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
>  goto efault;
> -target_fl->l_type = tswap16(fl.l_type);
> +target_fl->l_type = host_to_target_bitmask(
> + tswap16(fl.l_type), 
> flock_tbl);
>  target_fl->l_whence = tswap16(fl.l_whence);
>  target_fl->l_start = tswap64(fl.l_start);
>  target_fl->l_len = tswap64(fl.l_len);
> @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  if (((CPUARMState *)cpu_env)->eabi) {
>  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>  goto efault;
> -fl.l_type = tswap16(target_efl->l_type);
> +fl.l_type = 
> target_to_host_bitmask(tswap16(target_fl->l_type),
> +   flock_tbl);
>

Re: [Qemu-devel] [PATCH v3 3/4] target-tilegx: Add double floating point implementation

2015-12-11 Thread Chen Gang

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

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.

It we have to try to match the real hardware have done, I shall rewrite
the related code for mantissa. (I guess, we need to match the real
hardware have done).


> Do we know for sure that "7" is the correct number of guard bits?  From the 
> gcc
> implementation of floatsidf, I might guess that the correct number is "4".
> 

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   76   5   4   3   2   1   0
 *
 *1   0   0   0   0   1   1   0   1   1   00   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
 *
 */

I guess, I need restore this comments in helper_fdouble.c.


>> +static uint32_t get_fdouble_vexp(uint64_t n)
>> +{
>> +return extract32(n, 7, 13);
>> +}
> 
> What's a "vexp"?
> 

It is exp + overflow bit + underflow bit. We can use vexp for internal
calculation, directly, and check uv and ov for the result. I guess the
real hardware will do like this.

The full description of the format is:

typedef union TileGXFPDFmtF {

struct {
uint64_t unknown0 : 7;/* unknown */

uint64_t vexp : 13;  /* vexp = exp | ov | uv */
#if 0 /* it is only the explanation for vexp above */
uint64_t exp : 11;/* exp, 0x21b << 1: 55 + TILEGX_F_EXP_DZERO */
uint64_t ov : 1;  /* overflow for mul, low priority */
uint64_t uv : 1;  /* underflow for mul, high priority */
#endif

uint64_t sign : 1;/* Sign bit for the total value */

uint64_t calc: 2; /* absolute add, sub, or mul */
uint64_t inf: 1;  /* infinit */
uint64_t nan: 1;  /* nan */

/* Come from TILE-Gx ISA document, Table 7-2 for floating point */
uint64_t unordered : 1;   /* The two are unordered */
uint64_t lt : 1;  /* 1st is less than 2nd */
uint64_t le : 1;  /* 1st is less than or equal to 2nd */
uint64_t gt : 1;  /* 1st is greater than 2nd */
uint64_t ge : 1;  /* 1st is greater than or equal to 2nd */
uint64_t eq : 1;  /* The two operands are equal */
uint64_t neq : 1; /* The two operands are not equal */

uint64_t unknown1 : 32;   /* unknown */
} fmt;
uint64_t ll;  /* only for easy using */
} TileGXFPDFmtF;


>> +uint64_t helper_fdouble_unpack_min(CPUTLGState *env,
>> +   uint64_t srca, uint64_t srcb)
>> +{
>> +uint64_t v = 0;
>> +uint32_t expa = get_f64_exp(srca);
>> +uint32_t expb = get_f64_exp(srcb);
>> +
>> +if (float64_is_any_nan(srca) || float64_is_any_nan(srcb)
>> +|| float64_is_infinity(srca) || float64_is_infinity(srcb)) {
>> +return 0;
>> +} else if (expa > expb) {
>> +if (expa - expb < 64) {
>> +set_fdouble_man(, fr_to_man(srcb) >> (expa - expb));
>> +} else {
>> +return 0;
>> +}
>> +} else if (expa < expb) {
>> +if (expb - expa < 64) {
>> +set_fdouble_man(, fr_to_man(srca) >> (expb - expa));
> 
> I very sincerely doubt that a simple right-shift is correct.  In order to
> obtain proper rounding for real computation, a sticky bit is required.  That
> is, set bit 0 if any bits are shifted out.  See the implementation of
> shift64RightJamming in fpu/softfloat-macros.h.
> 

Oh, really, thanks.


>> +uint64_t helper_fdouble_addsub(CPUTLGState *env,
>> +   uint64_t dest, uint64_t srca, uint64_t srcb)
>> +{
>> +if (get_fdouble_calc(srcb) == TILEGX_

Re: [Qemu-devel] [PATCH v3 3/4] target-tilegx: Add double floating point implementation

2015-12-11 Thread Chen Gang

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   76   5   4   3   2   1   0
>>   *
>>   *1   0   0   0   0   1   1   0   1   1   00   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



[Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation

2015-12-10 Thread Chen Gang

It passes gcc testsuite.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/helper-fsingle.c | 212 +
 1 file changed, 212 insertions(+)
 create mode 100644 target-tilegx/helper-fsingle.c

diff --git a/target-tilegx/helper-fsingle.c b/target-tilegx/helper-fsingle.c
new file mode 100644
index 000..a33837e
--- /dev/null
+++ b/target-tilegx/helper-fsingle.c
@@ -0,0 +1,212 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "helper-fshared.c"
+
+/*
+ * FSingle instructions implemenation:
+ *
+ * fsingle_add1 ; calc srca and srcb,
+ *  ; convert float_32 to TileGXFPSFmt result.
+ *  ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_sub1 ; calc srca and srcb.
+ *  ; convert float_32 to TileGXFPSFmt result.
+ *  ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_addsub2  ; nop.
+ *
+ * fsingle_mul1 ; calc srca and srcb.
+ *  ; convert float_32 value to TileGXFPSFmt result.
+ *  ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_mul2 ; move srca to dest.
+ *
+ * fsingle_pack1; nop
+ *
+ * fsingle_pack2; treate srca as TileGXFPSFmt result.
+ *  ; convert TileGXFPSFmt result to float_32 value.
+ *  ; move float_32 value to dest.
+ */
+
+#define TILEGX_F_CALC_CVT   0 /* convert int to fsingle */
+#define TILEGX_F_CALC_NCVT  1 /* Not convertion */
+
+static uint32_t get_f32_exp(float32 f)
+{
+return extract32(float32_val(f), 23, 8);
+}
+
+static void set_f32_exp(float32 *f, uint32_t exp)
+{
+*f = make_float32(deposit32(float32_val(*f), 23, 8, exp));
+}
+
+static uint32_t get_f32_man(float32 f)
+{
+return float32_val(f) & 0x7f;
+}
+
+static float32 create_f32_man(uint32_t man)
+{
+ return make_float32(man & 0x7f);
+}
+
+static inline uint32_t get_fsingle_exp(uint64_t n)
+{
+return n & 0xff;
+}
+
+static inline uint64_t create_fsingle_exp(uint32_t exp)
+{
+return exp & 0xff;
+}
+
+static inline uint32_t get_fsingle_sign(uint64_t n)
+{
+return test_bit(10, );
+}
+
+static inline void set_fsingle_sign(uint64_t *n)
+{
+set_bit(10, n);
+}
+
+static inline unsigned int get_fsingle_calc(uint64_t n)
+{
+return test_bit(11, );
+}
+
+static inline void set_fsingle_calc(uint64_t *n, uint32_t calc)
+{
+set_bit(11, n);
+}
+
+static inline unsigned int get_fsingle_man(uint64_t n)
+{
+return n >> 32;
+}
+
+static inline uint64_t create_fsingle_man(uint32_t man)
+{
+return (uint64_t)man << 32;
+}
+
+static uint64_t float32_to_sfmt(float32 f)
+{
+uint64_t sfmt = 0;
+
+if (float32_is_neg(f)) {
+set_fsingle_sign();
+}
+sfmt |= create_fsingle_exp(get_f32_exp(f));
+sfmt |= create_fsingle_man((get_f32_man(f) << 8) | (1 << 31));
+
+return sfmt;
+}
+
+static float32 sfmt_to_float32(uint64_t sfmt, float_status *fp_status)
+{
+float32 f;
+uint32_t sign = get_fsingle_sign(sfmt);
+uint32_t man = get_fsingle_man(sfmt);
+
+if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) {
+if (sign) {
+return int32_to_float32(0 - man, fp_status);
+} else {
+return uint32_to_float32(man, fp_status);
+}
+} else {
+f = float32_set_sign(float32_zero, sign);
+f |= create_f32_man(man >> 8);
+set_f32_exp(, get_fsingle_exp(sfmt));
+}
+
+return f;
+}
+
+uint64_t helper_fsingle_pack2(CPUTLGState *env, uint64_t srca)
+{
+return float32_val(sfmt_to_float32(srca, >fp_status));
+}
+
+static void ana_bits(float_status *fp_status,
+ float32 fsrca, float32 fsrcb, uint64_t *sfmt)
+{
+if (float32_eq(fsrca, fsrcb, fp_status)) {
+*sfmt |= create_fsfd_flag_eq();
+} else {
+*sfmt |= create_fsfd_flag_ne();
+}
+
+if (float32_lt(fsrca, fsrcb, fp_status)) {
+*sfmt |= create_fsfd_flag_lt();
+}
+if (float32_le

[Qemu-devel] [PATCH v3 1/4] target-tilegx: Add floating point shared functions

2015-12-10 Thread Chen Gang

They are used by fsingle and fdouble helpers.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/helper-fshared.c | 53 ++
 1 file changed, 53 insertions(+)
 create mode 100644 target-tilegx/helper-fshared.c

diff --git a/target-tilegx/helper-fshared.c b/target-tilegx/helper-fshared.c
new file mode 100644
index 000..d669f58
--- /dev/null
+++ b/target-tilegx/helper-fshared.c
@@ -0,0 +1,53 @@
+/*
+ *  TILE-Gx virtual Floating point shared functions
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+static inline uint64_t create_fsfd_flag_un(void)
+{
+return 1 << 25;
+}
+
+static inline uint64_t create_fsfd_flag_lt(void)
+{
+return 1 << 26;
+}
+
+static inline uint64_t create_fsfd_flag_le(void)
+{
+return 1 << 27;
+}
+
+static inline uint64_t create_fsfd_flag_gt(void)
+{
+return 1 << 28;
+}
+
+static inline uint64_t create_fsfd_flag_ge(void)
+{
+return 1 << 29;
+}
+
+static inline uint64_t create_fsfd_flag_eq(void)
+{
+return 1 << 30;
+}
+
+static inline uint64_t create_fsfd_flag_ne(void)
+{
+return 1ULL << 31;
+}
-- 
1.9.3



Re: [Qemu-devel] [PATCH v3 4/4] target-tilegx: Integrate floating pointer implementation

2015-12-10 Thread Chen Gang
On 12/11/15 05:37, Richard Henderson wrote:
> On 12/10/2015 06:16 AM, Chen Gang wrote:

[...]

>>
>> diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
>> index 03df107..445a606 100644
>> --- a/target-tilegx/cpu.h
>> +++ b/target-tilegx/cpu.h
>> @@ -88,6 +88,8 @@ typedef struct CPUTLGState {
>>  uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside 
>> */
>>  uint64_t pc;   /* Current pc */
>>  
>> +float_status fp_status;/* floating point status */
> 
> As mentioned elsewhere, this is pointless.
>

OK, thanks.
 
>> +case OE_RR_X0(FSINGLE_PACK1):
>> +case OE_RR_Y0(FSINGLE_PACK1):
>> +mnemonic = "fsingle_pack1";
>> +goto done2;
> 
> This could use a comment that we're "copying" dest to dest.
> 

OK, thanks.

>> @@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, 
>> unsigned opext,
>>  static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
>> unsigned dest, unsigned srca, unsigned srcb)
>>  {
>> -TCGv tdest = dest_gr(dc, dest);
>> -TCGv tsrca = load_gr(dc, srca);
>> -TCGv tsrcb = load_gr(dc, srcb);
>> +TCGv tdest, tsrca, tsrcb;
>>  TCGv t0;
>>  const char *mnemonic;
>>  
>>  switch (opext) {
>> +case OE_RRR(FSINGLE_ADDSUB2, 0, X0):
>> +mnemonic = "fsingle_addsub2";
>> +goto done2;
>> +}
> 
> Likewise.
> 

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation

2015-12-10 Thread Chen Gang

On 12/11/15 04:18, Richard Henderson wrote:
> On 12/10/2015 09:15 AM, Richard Henderson wrote:
>>   d = (uint64_t)sign << 63;
>>   d = deposit64(d, 53, 11, exp);
>>   d = deposit64(d, 21, 32, man);
>>   return float64_to_float32(d, fp_status);
> 
> Hmm.  Actually, this incorrectly adds the implicit bit.  We'd actually need to
> steal portions of softfloat.c to do this properly.  Which still isn't that
> difficult.
> 

Yes, thanks.

-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v3 2/4] target-tilegx: Add single floating point implementation

2015-12-10 Thread Chen Gang

On 12/11/15 01:15, Richard Henderson wrote:
> On 12/10/2015 06:15 AM, Chen Gang wrote:
>> +#define TILEGX_F_CALC_CVT   0 /* convert int to fsingle */
>> +#define TILEGX_F_CALC_NCVT  1 /* Not convertion */
>> +
>> +static uint32_t get_f32_exp(float32 f)
>> +{
>> +return extract32(float32_val(f), 23, 8);
>> +}
>> +
>> +static void set_f32_exp(float32 *f, uint32_t exp)
>> +{
>> +*f = make_float32(deposit32(float32_val(*f), 23, 8, exp));
>> +}
> 
> Why take a pointer instead of returning the new value?
>

I referenced set_* functions' declarations in "include/fpu/softfloat.h",
originally.

 
>> +static inline uint32_t get_fsingle_sign(uint64_t n)
>> +{
>> +return test_bit(10, );
>> +}
>> +
>> +static inline void set_fsingle_sign(uint64_t *n)
>> +{
>> +set_bit(10, n);
>> +}
> 
> Why are you using test_bit and set_bit here, rather than continuing to use
> deposit and extract?
> 

It is really only for one bit test and set, so test_bit/set_bit are
simpler and clearer than deposit/extract.


>> +static float32 sfmt_to_float32(uint64_t sfmt, float_status *fp_status)
>> +{
>> +float32 f;
>> +uint32_t sign = get_fsingle_sign(sfmt);
>> +uint32_t man = get_fsingle_man(sfmt);
>> +
>> +if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) {
>> +if (sign) {
>> +return int32_to_float32(0 - man, fp_status);
>> +} else {
>> +return uint32_to_float32(man, fp_status);
>> +}
>> +} else {
>> +f = float32_set_sign(float32_zero, sign);
>> +f |= create_f32_man(man >> 8);
>> +set_f32_exp(, get_fsingle_exp(sfmt));
>> +}
> 
> I'm not especially keen on this calc bit.  I'd much rather that we always pack
> and round properly.
>

OK.
 
> In particular, if gcc decided to optimize fractional fixed-point types, it
> would do something very similar to the current floatsisf2 code sequence, 
> except
> that it wouldn't use 0x9e as the exponent; it would use something smaller, so
> that some number of low bits of the mantessa would be below the radix point.
> 

Oh, really.

> Therefore, I think that fsingle_pack2 should do the following: Take the
> (sign,exp,man) tuple and slot them into a double -- recall that a single only
> has 23 bits in its mantessa, and this temp format has 32 -- then convert the
> double to a single.  Pre-rounded single results from fsingle_* will be
> unchanged, while integer data that gcc has constructed will be properly 
> rounded.
> 
> E.g.
> 
>   uint32_t sign = get_fsingle_sign(sfmt);
>   uint32_t exp = get_fsingle_exp(sfmt);
>   uint32_t man = get_fsingle_man(sfmt);
>   uint64_t d;
> 
>   /* Adjust the exponent for double precision, preserving Inf/NaN.  */
>   if (exp == 0xff) {
> exp = 0x7ff;
>   } else {
> exp += 1023 - 127;
>   }
> 
>   d = (uint64_t)sign << 63;
>   d = deposit64(d, 53, 11, exp);
>   d = deposit64(d, 21, 32, man);
>   return float64_to_float32(d, fp_status);
> 
> Note that this does require float32_to_sfmt to store the mantissa
> left-justified. That is, not in bits [54-32] as you're doing now, but in bits
> [63-41].
> 

For me, it is a good idea! :-)


>> +static void ana_bits(float_status *fp_status,
>> + float32 fsrca, float32 fsrcb, uint64_t *sfmt)
> 
> Is "ana" supposed to be short for "analyze"?
>

Yes.
 
>> +{
>> +if (float32_eq(fsrca, fsrcb, fp_status)) {
>> +*sfmt |= create_fsfd_flag_eq();
>> +} else {
>> +*sfmt |= create_fsfd_flag_ne();
>> +}
>> +
>> +if (float32_lt(fsrca, fsrcb, fp_status)) {
>> +*sfmt |= create_fsfd_flag_lt();
>> +}
>> +if (float32_le(fsrca, fsrcb, fp_status)) {
>> +*sfmt |= create_fsfd_flag_le();
>> +}
>> +
>> +if (float32_lt(fsrcb, fsrca, fp_status)) {
>> +*sfmt |= create_fsfd_flag_gt();
>> +}
>> +if (float32_le(fsrcb, fsrca, fp_status)) {
>> +*sfmt |= create_fsfd_flag_ge();
>> +}
>> +
>> +if (float32_unordered(fsrca, fsrcb, fp_status)) {
>> +*sfmt |= create_fsfd_flag_un();
>> +}
>> +}
> 
> Again, I think it's better to return the new sfmt value than modify a pointer.
> 

Oh, I guess, we can inline ana_bits() to main_calc(), for they are both
simple short functions, and ana_bits() is only called by main_calc().

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH v3 0/4] target-tilegx: Implement floating point instructions

2015-12-10 Thread Chen Gang
Hello all:

After communicated with my company, I am permitted to use my company
email to send patches to open source community.

My company supports what I have done for open source community, but at
present, I still use my personal mail id as Signed-of-by (make and send
patches mainly during my free time).

And thank my company for the support. :-)


Thanks.

On 12/10/15 22:12, Chen Gang wrote:
> 
> These patches are the normal floating point implementation, instead of
> the original temporary one.
> 
> It passes building, and gcc testsuite.
> 
> Chen Gang (4):
>   target-tilegx: Add floating point shared functions
>   target-tilegx: Add single floating point implementation
>   target-tilegx: Add double floating point implementation
>   target-tilegx: Integrate floating pointer implementation
> 
>  target-tilegx/Makefile.objs|   3 +-
>  target-tilegx/cpu.h|   2 +
>  target-tilegx/helper-fdouble.c | 400 
> +
>  target-tilegx/helper-fshared.c |  53 ++
>  target-tilegx/helper-fsingle.c | 212 ++
>  target-tilegx/helper.h |  12 ++
>  target-tilegx/translate.c  |  68 ++-
>  7 files changed, 740 insertions(+), 10 deletions(-)
>  create mode 100644 target-tilegx/helper-fdouble.c
>  create mode 100644 target-tilegx/helper-fshared.c
>  create mode 100644 target-tilegx/helper-fsingle.c
> 

-- 
Chen Gang (陈刚)

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



[Qemu-devel] [PATCH v3 3/4] target-tilegx: Add double floating point implementation

2015-12-10 Thread Chen Gang

It passes gcc testsuite.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/helper-fdouble.c | 400 +
 1 file changed, 400 insertions(+)
 create mode 100644 target-tilegx/helper-fdouble.c

diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c
new file mode 100644
index 000..3b824f7
--- /dev/null
+++ b/target-tilegx/helper-fdouble.c
@@ -0,0 +1,400 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "helper-fshared.c"
+
+/*
+ * FDouble instructions implemenation:
+ *
+ * fdouble_unpack_min   ; srca and srcb are float_64 value.
+ *  ; get the min absolute value's mantissa.
+ *  ; move "mantissa >> (exp_max - exp_min)" to dest.
+ *
+ * fdouble_unpack_max   ; srca and srcb are float_64 value.
+ *  ; get the max absolute value's mantissa.
+ *  ; move mantissa to dest.
+ *
+ * fdouble_add_flags; srca and srcb are float_64 value.
+ *  ; calc exp (exp_max), sign, and comp bits for flags.
+ *  ; set addsub bit to flags and move flags to dest.
+ *
+ * fdouble_sub_flags; srca and srcb are float_64 value.
+ *  ; calc exp (exp_max), sign, and comp bits for flags.
+ *  ; set addsub bit to flags and move flags to dest.
+ *
+ * fdouble_addsub:  ; dest, srca (max, min mantissa), and srcb (flags).
+ *  ; "dest +/- srca" depend on the add/sub bit of flags.
+ *  ; move result mantissa to dest.
+ *
+ * fdouble_mul_flags:   ; srca and srcb are float_64 value.
+ *  ; calc sign (xor), exp (min + max), and comp bits.
+ *  ; mix sign, exp, and comp bits as flags to dest.
+ *
+ * fdouble_pack1; move srcb (flags) to dest.
+ *
+ * fdouble_pack2; srca, srcb (high, low mantissa), and dest (flags)
+ *  ; normalize and pack result from srca, srcb, and dest.
+ *  ; move result to dest.
+ */
+
+#define TILEGX_F_EXP_DZERO  0x3ff /* Zero exp for double 11-bits */
+#define TILEGX_F_EXP_DMAX   0x7fe /* max exp for double 11-bits */
+#define TILEGX_F_EXP_DUF0x1000/* underflow exp bit for double */
+
+#define TILEGX_F_MAN_HBIT   (1ULL << 59)
+
+#define TILEGX_F_CALC_ADD   1 /* Perform absolute add operation */
+#define TILEGX_F_CALC_SUB   2 /* Perform absolute sub operation */
+#define TILEGX_F_CALC_MUL   3 /* Perform absolute mul operation */
+
+static uint32_t get_f64_exp(float64 d)
+{
+return extract64(float64_val(d), 52, 11);
+}
+
+static void set_f64_exp(float64 *d, uint32_t exp)
+{
+*d = make_float64(deposit64(float64_val(*d), 52, 11, exp));
+}
+
+static uint64_t get_f64_man(float64 d)
+{
+return extract64(float64_val(d), 0, 52);
+}
+
+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;
+}
+
+static uint64_t get_fdouble_man(uint64_t n)
+{
+return extract64(n, 0, 60);
+}
+
+static void set_fdouble_man(uint64_t *n, uint64_t man)
+{
+*n = deposit64(*n, 0, 60, man);
+}
+
+static uint64_t get_fdouble_man_of(uint64_t n)
+{
+return test_bit(60, );
+}
+
+static void clear_fdouble_man_of(uint64_t *n)
+{
+return clear_bit(60, n);
+}
+
+static uint32_t get_fdouble_nan(uint64_t n)
+{
+return test_bit(24, );
+}
+
+static void set_fdouble_nan(uint64_t *n)
+{
+set_bit(24, n);
+}
+
+static uint32_t get_fdouble_inf(uint64_t n)
+{
+return test_bit(23, );
+}
+
+static void set_fdouble_inf(uint64_t *n)
+{
+set_bit(23, n);
+}
+
+static uint32_t get_fdouble_calc(uint64_t n)
+{
+return extract32(n, 21, 2);
+}
+
+static void set_fdouble_calc(uint64_t *n, uint32_t calc)
+{
+*n = deposit64(*n, 21, 2, calc);
+}
+
+static uint32_t get_fdouble_sign(uint64_t n)
+{
+return test_bit(20, );
+}
+
+static void set_fdouble_sign(uint64_t *n)
+{
+set_bit(20, n);
+}
+

[Qemu-devel] [PATCH v3 4/4] target-tilegx: Integrate floating pointer implementation

2015-12-10 Thread Chen Gang

It passes normal building, and gcc testsuite.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/Makefile.objs |  3 +-
 target-tilegx/cpu.h |  2 ++
 target-tilegx/helper.h  | 12 
 target-tilegx/translate.c   | 68 +++--
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
index 0db778f..136ad60 100644
--- a/target-tilegx/Makefile.objs
+++ b/target-tilegx/Makefile.objs
@@ -1 +1,2 @@
-obj-y += cpu.o translate.o helper.o simd_helper.o
+obj-y += cpu.o translate.o helper.o simd_helper.o \
+   helper-fsingle.o helper-fdouble.o
diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
index 03df107..445a606 100644
--- a/target-tilegx/cpu.h
+++ b/target-tilegx/cpu.h
@@ -88,6 +88,8 @@ typedef struct CPUTLGState {
 uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */
 uint64_t pc;   /* Current pc */
 
+float_status fp_status;/* floating point status */
+
 #if defined(CONFIG_USER_ONLY)
 uint64_t excaddr;  /* exception address */
 uint64_t atomic_srca;  /* Arguments to atomic "exceptions" */
diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
index 9281d0f..b785bf2 100644
--- a/target-tilegx/helper.h
+++ b/target-tilegx/helper.h
@@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+
+DEF_HELPER_3(fsingle_add1, i64, env, i64, i64)
+DEF_HELPER_3(fsingle_sub1, i64, env, i64, i64)
+DEF_HELPER_3(fsingle_mul1, i64, env, i64, i64)
+DEF_HELPER_2(fsingle_pack2, i64, env, i64)
+DEF_HELPER_3(fdouble_unpack_min, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_unpack_max, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_add_flags, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_sub_flags, i64, env, i64, i64)
+DEF_HELPER_4(fdouble_addsub, i64, env, i64, i64, i64)
+DEF_HELPER_3(fdouble_mul_flags, i64, env, i64, i64)
+DEF_HELPER_4(fdouble_pack2, i64, env, i64, i64, i64)
diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index 354f25a..5c2a98d 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 }
 qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]);
 return ret;
+
+case OE_RR_X0(FSINGLE_PACK1):
+case OE_RR_Y0(FSINGLE_PACK1):
+mnemonic = "fsingle_pack1";
+goto done2;
 }
 
 tdest = dest_gr(dc, dest);
@@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 gen_helper_cnttz(tdest, tsrca);
 mnemonic = "cnttz";
 break;
-case OE_RR_X0(FSINGLE_PACK1):
-case OE_RR_Y0(FSINGLE_PACK1):
-return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
 case OE_RR_X1(LD1S):
 memop = MO_SB;
 mnemonic = "ld1s"; /* prefetch_l1_fault */
@@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 return TILEGX_EXCP_OPCODE_UNKNOWN;
 }
 
+done2:
 qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic,
   reg_names[dest], reg_names[srca]);
 return ret;
@@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
unsigned dest, unsigned srca, unsigned srcb)
 {
-TCGv tdest = dest_gr(dc, dest);
-TCGv tsrca = load_gr(dc, srca);
-TCGv tsrcb = load_gr(dc, srcb);
+TCGv tdest, tsrca, tsrcb;
 TCGv t0;
 const char *mnemonic;
 
 switch (opext) {
+case OE_RRR(FSINGLE_ADDSUB2, 0, X0):
+mnemonic = "fsingle_addsub2";
+goto done2;
+}
+
+tdest = dest_gr(dc, dest);
+tsrca = load_gr(dc, srca);
+tsrcb = load_gr(dc, srcb);
+
+switch (opext) {
 case OE_RRR(ADDXSC, 0, X0):
 case OE_RRR(ADDXSC, 0, X1):
 gen_saturate_op(tdest, tsrca, tsrcb, tcg_gen_add_tl);
@@ -906,14 +917,39 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned 
opext,
 mnemonic = "exch";
 break;
 case OE_RRR(FDOUBLE_ADDSUB, 0, X0):
+gen_helper_fdouble_addsub(tdest, cpu_env,
+  load_gr(dc, dest), tsrca, tsrcb);
+mnemonic = "fdouble_addsub";
+break;
 case OE_RRR(FDOUBLE_ADD_FLAGS, 0, X0):
+gen_helper_fdouble_add_flags(tdest, cpu_env, tsrca, tsrcb);
+mnemonic = "fdouble_add_flags";
+break;
 case OE_RRR(FDOUBLE_MUL_FLAGS, 0, X0):
+gen_helper_fdouble_mul_flags(tdest, cpu_env, tsrca, tsrcb);
+mnemon

Re: [Qemu-devel] [PATCH v2 0/4] target-tilegx: Implement floating point instructions

2015-11-29 Thread Chen Gang
Hello Maintainers:

Please help check these patches when you have time.

If it is necessary to send patch v3 for it, please let me know.


Thanks.

On 11/17/15 03:37, Chen Gang wrote:
> From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5...@gmail.com>
> Date: Tue, 17 Nov 2015 03:13:54 +0800
> Subject: [PATCH v2 0/4] target-tilegx: Implement floating point instructions
>
> These patches are the normal floating point implementation, instead of
> the original temporary one.
>
> It passes building, and gcc testsuite.
>
> Chen Gang (4):
> target-tilegx: Add floating point shared functions
> target-tilegx: Add single floating point implementation
> target-tilegx: Add double floating point implementation
> target-tilegx: Integrate floating pointer implementation
>
> target-tilegx/Makefile.objs | 3 +-
> target-tilegx/cpu.h | 2 +
> target-tilegx/helper-fdouble.c | 400 +
> target-tilegx/helper-fshared.c | 53 ++
> target-tilegx/helper-fsingle.c | 212 ++
> target-tilegx/helper.h | 12 ++
> target-tilegx/translate.c | 68 ++-
> 7 files changed, 740 insertions(+), 10 deletions(-)
> create mode 100644 target-tilegx/helper-fdouble.c
> create mode 100644 target-tilegx/helper-fshared.c
> create mode 100644 target-tilegx/helper-fsingle.c
>
> --
> 1.9.3
>

--
Chen Gang (陈刚)

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

Re: [Qemu-devel] [PATCH v2 3/4] target-tilegx: Add double floating point implementation

2015-11-16 Thread Chen Gang
Oh, sorry again, the original attachment is incorrect, either.

The attachment in this reply is the correct one.

Thanks.
--
Chen Gang

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



> From: xili_gchen_5...@hotmail.com
> To: r...@twiddle.net; peter.mayd...@linaro.org; cmetc...@ezchip.com
> CC: qemu-devel@nongnu.org
> Subject: Re: [PATCH v2 3/4] target-tilegx: Add double floating point 
> implementation
> Date: Tue, 17 Nov 2015 03:57:28 +0800
>
>
> Excuse me, I copied/pasted the patch to the website, which may generate
> the incorrect coding styles.
>
> Please check the attachment in this reply mail for the correct coding
> styles (and sorry, the original mail's attachment which I chose is
> incorrect).
>
> On 11/17/15 03:41, Chen Gang wrote:
>> From db171c94e8c9df446c091d9f42004c80ed8c6ccc Mon Sep 17 00:00:00 2001
>> From: Chen Gang <gang.chen.5...@gmail.com>
>> Date: Tue, 17 Nov 2015 03:08:38 +0800
>> Subject: [PATCH v2 3/4] target-tilegx: Add double floating point 
>> implementation
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>> target-tilegx/helper-fdouble.c | 400 
>> +
>> 1 file changed, 400 insertions(+)
>> create mode 100644 target-tilegx/helper-fdouble.c
>>
>> diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c
>> new file mode 100644
>> index 000..f005c80
>> --- /dev/null
>> +++ b/target-tilegx/helper-fdouble.c
>> @@ -0,0 +1,400 @@
>> +/*
>> + * QEMU TILE-Gx helpers
>> + *
>> + * Copyright (c) 2015 Chen Gang
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see
>> + * <http://www.gnu.org/licenses/lgpl-2.1.html>
>> + */
>> +
>> +#include "cpu.h"
>> +#include "qemu-common.h"
>> +#include "exec/helper-proto.h"
>> +#include "fpu/softfloat.h"
>> +
>> +#include "helper-fshared.c"
>> +
>> +/*
>> + * FDouble instructions implemenation:
>> + *
>> + * fdouble_unpack_min ; srca and srcb are float_64 value.
>> + * ; get the min absolute value's mantissa.
>> + * ; move "mantissa>> (exp_max - exp_min)" to dest.
>> + *
>> + * fdouble_unpack_max ; srca and srcb are float_64 value.
>> + * ; get the max absolute value's mantissa.
>> + * ; move mantissa to dest.
>> + *
>> + * fdouble_add_flags ; srca and srcb are float_64 value.
>> + * ; calc exp (exp_max), sign, and comp bits for flags.
>> + * ; set addsub bit to flags and move flags to dest.
>> + *
>> + * fdouble_sub_flags ; srca and srcb are float_64 value.
>> + * ; calc exp (exp_max), sign, and comp bits for flags.
>> + * ; set addsub bit to flags and move flags to dest.
>> + *
>> + * fdouble_addsub: ; dest, srca (max, min mantissa), and srcb (flags).
>> + * ; "dest +/- srca" depend on the add/sub bit of flags.
>> + * ; move result mantissa to dest.
>> + *
>> + * fdouble_mul_flags: ; srca and srcb are float_64 value.
>> + * ; calc sign (xor), exp (min + max), and comp bits.
>> + * ; mix sign, exp, and comp bits as flags to dest.
>> + *
>> + * fdouble_pack1 ; move srcb (flags) to dest.
>> + *
>> + * fdouble_pack2 ; srca, srcb (high, low mantissa), and dest (flags)
>> + * ; normalize and pack result from srca, srcb, and dest.
>> + * ; move result to dest.
>> + */
>> +
>> +#define TILEGX_F_EXP_DZERO 0x3ff /* Zero exp for double 11-bits */
>> +#define TILEGX_F_EXP_DMAX 0x7fe /* max exp for double 11-bits */
>> +#define TILEGX_F_EXP_DUF 0x1000/* underflow exp bit for double */
>> +
>> +#define TILEGX_F_MAN_HBIT (1ULL << 59)
>> +
>> +#define TILEGX_F_CALC_ADD 1 /* Perform absolute add operation */
>> +#define TILEGX_F_CALC_SUB 2 /* Perform absolute sub operation */
>> +#define TILEGX_F_CALC_MUL 3 /* Perform ab

[Qemu-devel] [PATCH v2 2/4] target-tilegx: Add single floating point implementation

2015-11-16 Thread Chen Gang
>From 1c4bb91e72995a1675d3aa0f911c534a3e8db749 Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Tue, 17 Nov 2015 03:07:33 +0800
Subject: [PATCH v2 2/4] target-tilegx: Add single floating point implementation

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/helper-fsingle.c | 212 +
 1 file changed, 212 insertions(+)
 create mode 100644 target-tilegx/helper-fsingle.c

diff --git a/target-tilegx/helper-fsingle.c b/target-tilegx/helper-fsingle.c
new file mode 100644
index 000..a33837e
--- /dev/null
+++ b/target-tilegx/helper-fsingle.c
@@ -0,0 +1,212 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "helper-fshared.c"
+
+/*
+ * FSingle instructions implemenation:
+ *
+ * fsingle_add1         ; calc srca and srcb,
+ *                      ; convert float_32 to TileGXFPSFmt result.
+ *                      ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_sub1         ; calc srca and srcb.
+ *                      ; convert float_32 to TileGXFPSFmt result.
+ *                      ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_addsub2      ; nop.
+ *
+ * fsingle_mul1         ; calc srca and srcb.
+ *                      ; convert float_32 value to TileGXFPSFmt result.
+ *                      ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_mul2         ; move srca to dest.
+ *
+ * fsingle_pack1        ; nop
+ *
+ * fsingle_pack2        ; treate srca as TileGXFPSFmt result.
+ *                      ; convert TileGXFPSFmt result to float_32 value.
+ *                      ; move float_32 value to dest.
+ */
+
+#define TILEGX_F_CALC_CVT   0     /* convert int to fsingle */
+#define TILEGX_F_CALC_NCVT  1     /* Not convertion */
+
+static uint32_t get_f32_exp(float32 f)
+{
+    return extract32(float32_val(f), 23, 8);
+}
+
+static void set_f32_exp(float32 *f, uint32_t exp)
+{
+    *f = make_float32(deposit32(float32_val(*f), 23, 8, exp));
+}
+
+static uint32_t get_f32_man(float32 f)
+{
+    return float32_val(f) & 0x7f;
+}
+
+static float32 create_f32_man(uint32_t man)
+{
+     return make_float32(man & 0x7f);
+}
+
+static inline uint32_t get_fsingle_exp(uint64_t n)
+{
+    return n & 0xff;
+}
+
+static inline uint64_t create_fsingle_exp(uint32_t exp)
+{
+    return exp & 0xff;
+}
+
+static inline uint32_t get_fsingle_sign(uint64_t n)
+{
+    return test_bit(10, );
+}
+
+static inline void set_fsingle_sign(uint64_t *n)
+{
+    set_bit(10, n);
+}
+
+static inline unsigned int get_fsingle_calc(uint64_t n)
+{
+    return test_bit(11, );
+}
+
+static inline void set_fsingle_calc(uint64_t *n, uint32_t calc)
+{
+    set_bit(11, n);
+}
+
+static inline unsigned int get_fsingle_man(uint64_t n)
+{
+    return n>> 32;
+}
+
+static inline uint64_t create_fsingle_man(uint32_t man)
+{
+    return (uint64_t)man << 32;
+}
+
+static uint64_t float32_to_sfmt(float32 f)
+{
+    uint64_t sfmt = 0;
+
+    if (float32_is_neg(f)) {
+        set_fsingle_sign();
+    }
+    sfmt |= create_fsingle_exp(get_f32_exp(f));
+    sfmt |= create_fsingle_man((get_f32_man(f) << 8) | (1 << 31));
+
+    return sfmt;
+}
+
+static float32 sfmt_to_float32(uint64_t sfmt, float_status *fp_status)
+{
+    float32 f;
+    uint32_t sign = get_fsingle_sign(sfmt);
+    uint32_t man = get_fsingle_man(sfmt);
+
+    if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) {
+        if (sign) {
+            return int32_to_float32(0 - man, fp_status);
+        } else {
+            return uint32_to_float32(man, fp_status);
+        }
+    } else {
+        f = float32_set_sign(float32_zero, sign);
+        f |= create_f32_man(man>> 8);
+        set_f32_exp(, get_fsingle_exp(sfmt));
+    }
+
+    return f;
+}
+
+uint64_t helper_fsingle_pack2(CPUTLGState *env, uint64_t srca)
+{
+    return float32_val(sfmt_to_float32(srca, >fp_status));
+}
+
+static void ana_bits(float_status *fp_status,
+                     float32 fsrca, float32 fsrcb, uint64_t *sfmt)
+{
+    if (float32_eq(fsrca, fsrcb, fp_status)) {
+        *

Re: [Qemu-devel] [PATCH v2 4/4] target-tilegx: Integrate floating pointer implementation

2015-11-16 Thread Chen Gang

Excuse me, I copied/pasted the patch to the website, which may generate
the incorrect coding styles.

Please check the attachment in this reply mail for the correct coding
styles (and sorry, the original mail's attachment which I chose is
incorrect).


On 11/17/15 03:43, Chen Gang wrote:
> From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5...@gmail.com>
> Date: Tue, 17 Nov 2015 03:09:18 +0800
> Subject: [PATCH v2 4/4] target-tilegx: Integrate floating pointer 
> implementation
>
> It passes normal building, and gcc testsuite.
>
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
> target-tilegx/Makefile.objs | 3 +-
> target-tilegx/cpu.h | 2 ++
> target-tilegx/helper.h | 12 
> target-tilegx/translate.c | 68 +++--
> 4 files changed, 75 insertions(+), 10 deletions(-)
>
> diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
> index 0db778f..136ad60 100644
> --- a/target-tilegx/Makefile.objs
> +++ b/target-tilegx/Makefile.objs
> @@ -1 +1,2 @@
> -obj-y += cpu.o translate.o helper.o simd_helper.o
> +obj-y += cpu.o translate.o helper.o simd_helper.o \
> + helper-fsingle.o helper-fdouble.o
> diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
> index 03df107..445a606 100644
> --- a/target-tilegx/cpu.h
> +++ b/target-tilegx/cpu.h
> @@ -88,6 +88,8 @@ typedef struct CPUTLGState {
> uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */
> uint64_t pc; /* Current pc */
>
> + float_status fp_status; /* floating point status */
> +
> #if defined(CONFIG_USER_ONLY)
> uint64_t excaddr; /* exception address */
> uint64_t atomic_srca; /* Arguments to atomic "exceptions" */
> diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
> index 9281d0f..b785bf2 100644
> --- a/target-tilegx/helper.h
> +++ b/target-tilegx/helper.h
> @@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, 
> i64)
> DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64)
> DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64)
> DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
> +
> +DEF_HELPER_3(fsingle_add1, i64, env, i64, i64)
> +DEF_HELPER_3(fsingle_sub1, i64, env, i64, i64)
> +DEF_HELPER_3(fsingle_mul1, i64, env, i64, i64)
> +DEF_HELPER_2(fsingle_pack2, i64, env, i64)
> +DEF_HELPER_3(fdouble_unpack_min, i64, env, i64, i64)
> +DEF_HELPER_3(fdouble_unpack_max, i64, env, i64, i64)
> +DEF_HELPER_3(fdouble_add_flags, i64, env, i64, i64)
> +DEF_HELPER_3(fdouble_sub_flags, i64, env, i64, i64)
> +DEF_HELPER_4(fdouble_addsub, i64, env, i64, i64, i64)
> +DEF_HELPER_3(fdouble_mul_flags, i64, env, i64, i64)
> +DEF_HELPER_4(fdouble_pack2, i64, env, i64, i64, i64)
> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
> index b8ca401..4d74b1d 100644
> --- a/target-tilegx/translate.c
> +++ b/target-tilegx/translate.c
> @@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
> opext,
> }
> qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]);
> return ret;
> +
> + case OE_RR_X0(FSINGLE_PACK1):
> + case OE_RR_Y0(FSINGLE_PACK1):
> + mnemonic = "fsingle_pack1";
> + goto done2;
> }
>
> tdest = dest_gr(dc, dest);
> @@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
> opext,
> gen_helper_cnttz(tdest, tsrca);
> mnemonic = "cnttz";
> break;
> - case OE_RR_X0(FSINGLE_PACK1):
> - case OE_RR_Y0(FSINGLE_PACK1):
> - return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
> case OE_RR_X1(LD1S):
> memop = MO_SB;
> mnemonic = "ld1s"; /* prefetch_l1_fault */
> @@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
> opext,
> return TILEGX_EXCP_OPCODE_UNKNOWN;
> }
>
> +done2:
> qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic,
> reg_names[dest], reg_names[srca]);
> return ret;
> @@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, 
> unsigned opext,
> static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
> unsigned dest, unsigned srca, unsigned srcb)
> {
> - TCGv tdest = dest_gr(dc, dest);
> - TCGv tsrca = load_gr(dc, srca);
> - TCGv tsrcb = load_gr(dc, srcb);
> + TCGv tdest, tsrca, tsrcb;
> TCGv t0;
> const char *mnemonic;
>
> switch (opext) {
> + case OE_RRR(FSINGLE_ADDSUB2, 0, X0):
> + mnemonic = "fsingle_addsub2";
> + goto done2;
> + }
> +
> + tdest = dest_gr(dc, dest);
> + tsrca = load_gr(dc, srca);
> + tsrcb = load_gr(dc, srcb);
> +
> + switch (opext) {
> case OE_RRR(ADDXSC, 0, X0):
> case OE_RRR(ADDXSC, 0, X1):
>

[Qemu-devel] [PATCH v2 4/4] target-tilegx: Integrate floating pointer implementation

2015-11-16 Thread Chen Gang
>From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Tue, 17 Nov 2015 03:09:18 +0800
Subject: [PATCH v2 4/4] target-tilegx: Integrate floating pointer implementation

It passes normal building, and gcc testsuite.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/Makefile.objs |  3 +-
 target-tilegx/cpu.h         |  2 ++
 target-tilegx/helper.h      | 12 
 target-tilegx/translate.c   | 68 +++--
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
index 0db778f..136ad60 100644
--- a/target-tilegx/Makefile.objs
+++ b/target-tilegx/Makefile.objs
@@ -1 +1,2 @@
-obj-y += cpu.o translate.o helper.o simd_helper.o
+obj-y += cpu.o translate.o helper.o simd_helper.o \
+   helper-fsingle.o helper-fdouble.o
diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
index 03df107..445a606 100644
--- a/target-tilegx/cpu.h
+++ b/target-tilegx/cpu.h
@@ -88,6 +88,8 @@ typedef struct CPUTLGState {
     uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */
     uint64_t pc;                       /* Current pc */
 
+    float_status fp_status;            /* floating point status */
+
 #if defined(CONFIG_USER_ONLY)
     uint64_t excaddr;                  /* exception address */
     uint64_t atomic_srca;              /* Arguments to atomic "exceptions" */
diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
index 9281d0f..b785bf2 100644
--- a/target-tilegx/helper.h
+++ b/target-tilegx/helper.h
@@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+
+DEF_HELPER_3(fsingle_add1, i64, env, i64, i64)
+DEF_HELPER_3(fsingle_sub1, i64, env, i64, i64)
+DEF_HELPER_3(fsingle_mul1, i64, env, i64, i64)
+DEF_HELPER_2(fsingle_pack2, i64, env, i64)
+DEF_HELPER_3(fdouble_unpack_min, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_unpack_max, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_add_flags, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_sub_flags, i64, env, i64, i64)
+DEF_HELPER_4(fdouble_addsub, i64, env, i64, i64, i64)
+DEF_HELPER_3(fdouble_mul_flags, i64, env, i64, i64)
+DEF_HELPER_4(fdouble_pack2, i64, env, i64, i64, i64)
diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index b8ca401..4d74b1d 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
         }
         qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]);
         return ret;
+
+    case OE_RR_X0(FSINGLE_PACK1):
+    case OE_RR_Y0(FSINGLE_PACK1):
+        mnemonic = "fsingle_pack1";
+        goto done2;
     }
 
     tdest = dest_gr(dc, dest);
@@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
         gen_helper_cnttz(tdest, tsrca);
         mnemonic = "cnttz";
         break;
-    case OE_RR_X0(FSINGLE_PACK1):
-    case OE_RR_Y0(FSINGLE_PACK1):
-        return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
     case OE_RR_X1(LD1S):
         memop = MO_SB;
         mnemonic = "ld1s"; /* prefetch_l1_fault */
@@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
         return TILEGX_EXCP_OPCODE_UNKNOWN;
     }
 
+done2:
     qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic,
                   reg_names[dest], reg_names[srca]);
     return ret;
@@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
                                unsigned dest, unsigned srca, unsigned srcb)
 {
-    TCGv tdest = dest_gr(dc, dest);
-    TCGv tsrca = load_gr(dc, srca);
-    TCGv tsrcb = load_gr(dc, srcb);
+    TCGv tdest, tsrca, tsrcb;
     TCGv t0;
     const char *mnemonic;
 
     switch (opext) {
+    case OE_RRR(FSINGLE_ADDSUB2, 0, X0):
+        mnemonic = "fsingle_addsub2";
+        goto done2;
+    }
+
+    tdest = dest_gr(dc, dest);
+    tsrca = load_gr(dc, srca);
+    tsrcb = load_gr(dc, srcb);
+
+    switch (opext) {
     case OE_RRR(ADDXSC, 0, X0):
     case OE_RRR(ADDXSC, 0, X1):
         gen_saturate_op(tdest, tsrca, tsrcb, tcg_gen_add_tl);
@@ -906,14 +917,39 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned 
opext,
         mnemonic = "exch";
         break;
     case OE_RRR(FDOUBLE_ADDSUB, 0, X0):
+        gen_helper_fdouble_addsub(tdest, cpu_env,
+                                  load_gr(dc, dest),tsrca, tsrcb);
+        mnemonic = "fdouble_addsub";
+        break;
     case OE_RRR(FDOUBLE_ADD_FLAGS, 0, X0):
+        gen_helper

[Qemu-devel] [PATCH v2 3/4] target-tilegx: Add double floating point implementation

2015-11-16 Thread Chen Gang
>From db171c94e8c9df446c091d9f42004c80ed8c6ccc Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Tue, 17 Nov 2015 03:08:38 +0800
Subject: [PATCH v2 3/4] target-tilegx: Add double floating point implementation

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/helper-fdouble.c | 400 +
 1 file changed, 400 insertions(+)
 create mode 100644 target-tilegx/helper-fdouble.c

diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c
new file mode 100644
index 000..f005c80
--- /dev/null
+++ b/target-tilegx/helper-fdouble.c
@@ -0,0 +1,400 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "helper-fshared.c"
+
+/*
+ * FDouble instructions implemenation:
+ *
+ * fdouble_unpack_min   ; srca and srcb are float_64 value.
+ *                      ; get the min absolute value's mantissa.
+ *                      ; move "mantissa>> (exp_max - exp_min)" to dest.
+ *
+ * fdouble_unpack_max   ; srca and srcb are float_64 value.
+ *                      ; get the max absolute value's mantissa.
+ *                      ; move mantissa to dest.
+ *
+ * fdouble_add_flags    ; srca and srcb are float_64 value.
+ *                      ; calc exp (exp_max), sign, and comp bits for flags.
+ *                      ; set addsub bit to flags and move flags to dest.
+ *
+ * fdouble_sub_flags    ; srca and srcb are float_64 value.
+ *                      ; calc exp (exp_max), sign, and comp bits for flags.
+ *                      ; set addsub bit to flags and move flags to dest.
+ *
+ * fdouble_addsub:      ; dest, srca (max, min mantissa), and srcb (flags).
+ *                      ; "dest +/- srca" depend on the add/sub bit of flags.
+ *                      ; move result mantissa to dest.
+ *
+ * fdouble_mul_flags:   ; srca and srcb are float_64 value.
+ *                      ; calc sign (xor), exp (min + max), and comp bits.
+ *                      ; mix sign, exp, and comp bits as flags to dest.
+ *
+ * fdouble_pack1        ; move srcb (flags) to dest.
+ *
+ * fdouble_pack2        ; srca, srcb (high, low mantissa), and dest (flags)
+ *                      ; normalize and pack result from srca, srcb, and dest.
+ *                      ; move result to dest.
+ */
+
+#define TILEGX_F_EXP_DZERO  0x3ff /* Zero exp for double 11-bits */
+#define TILEGX_F_EXP_DMAX   0x7fe /* max exp for double 11-bits */
+#define TILEGX_F_EXP_DUF    0x1000/* underflow exp bit for double */
+
+#define TILEGX_F_MAN_HBIT   (1ULL << 59)
+
+#define TILEGX_F_CALC_ADD   1     /* Perform absolute add operation */
+#define TILEGX_F_CALC_SUB   2     /* Perform absolute sub operation */
+#define TILEGX_F_CALC_MUL   3     /* Perform absolute mul operation */
+
+static uint32_t get_f64_exp(float64 d)
+{
+    return extract64(float64_val(d), 52, 11);
+}
+
+static void set_f64_exp(float64 *d, uint32_t exp)
+{
+    *d = make_float64(deposit64(float64_val(*d), 52, 11, exp));
+}
+
+static uint64_t get_f64_man(float64 d)
+{
+    return extract64(float64_val(d), 0, 52);
+}
+
+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;
+}
+
+static uint64_t get_fdouble_man(uint64_t n)
+{
+    return extract64(n, 0, 60);
+}
+
+static void set_fdouble_man(uint64_t *n, uint64_t man)
+{
+    *n = deposit64(*n, 0, 60, man);
+}
+
+static uint64_t get_fdouble_man_of(uint64_t n)
+{
+    return test_bit(60, );
+}
+
+static void clear_fdouble_man_of(uint64_t *n)
+{
+    return clear_bit(60, n);
+}
+
+static uint32_t get_fdouble_nan(uint64_t n)
+{
+    return test_bit(24, );
+}
+
+static void set_fdouble_nan(uint64_t *n)
+{
+    set_bit(24, n);
+}
+
+static uint32_t get_fdouble_inf(uint64_t n)
+{
+    return test_bit(23, );
+}
+
+static void set_fdouble_inf(uint64_t *n)
+{
+    set_bit(23, n);
+}
+
+static uint32_t get_fdouble_calc(uint64_t n)
+{
+    return extract32(n, 21, 2);
+}
+
+static void set_fdouble_calc(uint64_t 

[Qemu-devel] [PATCH v2 0/4] target-tilegx: Implement floating point instructions

2015-11-16 Thread Chen Gang
>From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Tue, 17 Nov 2015 03:13:54 +0800
Subject: [PATCH v2 0/4] target-tilegx: Implement floating point instructions

These patches are the normal floating point implementation, instead of
the original temporary one.

It passes building, and gcc testsuite.

Chen Gang (4):
  target-tilegx: Add floating point shared functions
  target-tilegx: Add single floating point implementation
  target-tilegx: Add double floating point implementation
  target-tilegx: Integrate floating pointer implementation

 target-tilegx/Makefile.objs    |   3 +-
 target-tilegx/cpu.h            |   2 +
 target-tilegx/helper-fdouble.c | 400 +
 target-tilegx/helper-fshared.c |  53 ++
 target-tilegx/helper-fsingle.c | 212 ++
 target-tilegx/helper.h         |  12 ++
 target-tilegx/translate.c      |  68 ++-
 7 files changed, 740 insertions(+), 10 deletions(-)
 create mode 100644 target-tilegx/helper-fdouble.c
 create mode 100644 target-tilegx/helper-fshared.c
 create mode 100644 target-tilegx/helper-fsingle.c

-- 
1.9.3 

v2--cover-letter.patch
Description: Binary data


[Qemu-devel] [PATCH v2 1/4] target-tilegx: Add floating point shared functions

2015-11-16 Thread Chen Gang
>From 40c3e3f79b206b9506e0d6679e301885bb3ee277 Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Tue, 17 Nov 2015 03:04:50 +0800
Subject: [PATCH v2 1/4] target-tilegx: Add floating point shared functions

They are used by fsingle and fdouble helpers.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/helper-fshared.c | 53 ++
 1 file changed, 53 insertions(+)
 create mode 100644 target-tilegx/helper-fshared.c

diff --git a/target-tilegx/helper-fshared.c b/target-tilegx/helper-fshared.c
new file mode 100644
index 000..d669f58
--- /dev/null
+++ b/target-tilegx/helper-fshared.c
@@ -0,0 +1,53 @@
+/*
+ *  TILE-Gx virtual Floating point shared functions
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+static inline uint64_t create_fsfd_flag_un(void)
+{
+    return 1 << 25;
+}
+
+static inline uint64_t create_fsfd_flag_lt(void)
+{
+    return 1 << 26;
+}
+
+static inline uint64_t create_fsfd_flag_le(void)
+{
+    return 1 << 27;
+}
+
+static inline uint64_t create_fsfd_flag_gt(void)
+{
+    return 1 << 28;
+}
+
+static inline uint64_t create_fsfd_flag_ge(void)
+{
+    return 1 << 29;
+}
+
+static inline uint64_t create_fsfd_flag_eq(void)
+{
+    return 1 << 30;
+}
+
+static inline uint64_t create_fsfd_flag_ne(void)
+{
+    return 1ULL << 31;
+}
-- 
1.9.3

  

v2-0001-target-tilegx-Add-floating-point-shared-functions.patch
Description: Binary data


Re: [Qemu-devel] [PATCH v2 3/4] target-tilegx: Add double floating point implementation

2015-11-16 Thread Chen Gang

Excuse me, I copied/pasted the patch to the website, which may generate
the incorrect coding styles.

Please check the attachment in this reply mail for the correct coding
styles (and sorry, the original mail's attachment which I chose is
incorrect).

On 11/17/15 03:41, Chen Gang wrote:
> From db171c94e8c9df446c091d9f42004c80ed8c6ccc Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5...@gmail.com>
> Date: Tue, 17 Nov 2015 03:08:38 +0800
> Subject: [PATCH v2 3/4] target-tilegx: Add double floating point 
> implementation
>
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
> target-tilegx/helper-fdouble.c | 400 +
> 1 file changed, 400 insertions(+)
> create mode 100644 target-tilegx/helper-fdouble.c
>
> diff --git a/target-tilegx/helper-fdouble.c b/target-tilegx/helper-fdouble.c
> new file mode 100644
> index 000..f005c80
> --- /dev/null
> +++ b/target-tilegx/helper-fdouble.c
> @@ -0,0 +1,400 @@
> +/*
> + * QEMU TILE-Gx helpers
> + *
> + * Copyright (c) 2015 Chen Gang
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see
> + * <http://www.gnu.org/licenses/lgpl-2.1.html>
> + */
> +
> +#include "cpu.h"
> +#include "qemu-common.h"
> +#include "exec/helper-proto.h"
> +#include "fpu/softfloat.h"
> +
> +#include "helper-fshared.c"
> +
> +/*
> + * FDouble instructions implemenation:
> + *
> + * fdouble_unpack_min ; srca and srcb are float_64 value.
> + * ; get the min absolute value's mantissa.
> + * ; move "mantissa>> (exp_max - exp_min)" to dest.
> + *
> + * fdouble_unpack_max ; srca and srcb are float_64 value.
> + * ; get the max absolute value's mantissa.
> + * ; move mantissa to dest.
> + *
> + * fdouble_add_flags ; srca and srcb are float_64 value.
> + * ; calc exp (exp_max), sign, and comp bits for flags.
> + * ; set addsub bit to flags and move flags to dest.
> + *
> + * fdouble_sub_flags ; srca and srcb are float_64 value.
> + * ; calc exp (exp_max), sign, and comp bits for flags.
> + * ; set addsub bit to flags and move flags to dest.
> + *
> + * fdouble_addsub: ; dest, srca (max, min mantissa), and srcb (flags).
> + * ; "dest +/- srca" depend on the add/sub bit of flags.
> + * ; move result mantissa to dest.
> + *
> + * fdouble_mul_flags: ; srca and srcb are float_64 value.
> + * ; calc sign (xor), exp (min + max), and comp bits.
> + * ; mix sign, exp, and comp bits as flags to dest.
> + *
> + * fdouble_pack1 ; move srcb (flags) to dest.
> + *
> + * fdouble_pack2 ; srca, srcb (high, low mantissa), and dest (flags)
> + * ; normalize and pack result from srca, srcb, and dest.
> + * ; move result to dest.
> + */
> +
> +#define TILEGX_F_EXP_DZERO 0x3ff /* Zero exp for double 11-bits */
> +#define TILEGX_F_EXP_DMAX 0x7fe /* max exp for double 11-bits */
> +#define TILEGX_F_EXP_DUF 0x1000/* underflow exp bit for double */
> +
> +#define TILEGX_F_MAN_HBIT (1ULL << 59)
> +
> +#define TILEGX_F_CALC_ADD 1 /* Perform absolute add operation */
> +#define TILEGX_F_CALC_SUB 2 /* Perform absolute sub operation */
> +#define TILEGX_F_CALC_MUL 3 /* Perform absolute mul operation */
> +
> +static uint32_t get_f64_exp(float64 d)
> +{
> + return extract64(float64_val(d), 52, 11);
> +}
> +
> +static void set_f64_exp(float64 *d, uint32_t exp)
> +{
> + *d = make_float64(deposit64(float64_val(*d), 52, 11, exp));
> +}
> +
> +static uint64_t get_f64_man(float64 d)
> +{
> + return extract64(float64_val(d), 0, 52);
> +}
> +
> +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;
> +}
> +
> +static uint64_t get_fdouble_man(uint64_t n)
> +{
> + return extract64(n, 0, 60);
> +}
> +
> +static void set_fdouble_man(uint64_t *n, uint64_t man)
> +{
> + *n = deposit64(*n, 0, 60, man);
> +}
> +
> +static uint64_t get_fdouble_man_of(uint64_t n)
> +{
> + return test_

Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file

2015-11-12 Thread Chen Gang
On 11/12/15 22:34, Richard Henderson wrote:
> On 11/08/2015 06:43 AM, Chen Gang wrote:
> 
>> +#if !defined(HOST_WORDS_BIGENDIAN)
>> +/* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */
>> +uint64_t exp : 8; /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */
>> +uint64_t uiknown0 : 2;/* unknown */
> 
> I would really rather you didn't use bitfields, because of exactly this sort 
> of endianness problem.  Because, really, you can't trust this layout.  But I 
> won't press this point, because it is complicated enough already.
> 

Because of endianess issues, for me, I don't like bit fields either. But
I can not find any other simpler ways than current.

>> +#endif
>> +} TileGXFPSFmt;
>> +/*
> 
> Watch your spacing.
> 

OK, thanks. And I also shall let the related comments above the structure.


>> + * Double exp analyzing: (0x21b00 << 1) - 0x36(54) = 0x400

Oh, it should be (0x21b00 << 1) - 0x37(55) = 0x3ff

>> + *
>> + *   17  16  15  14  13  12  11  10   9   8   76   5   4   3   2   1   0
>> + *
>> + *1   0   0   0   0   1   1   0   1   1   00   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
> 
> What is this table supposed to mean?
> 

I want to use this table to explain my guess: at first, we don't know
the internal exp position, neither know the internal mantasa bits. We
have to guess both of them:

 - the real exp should not be less than 11 bits (IEEE double exp is 11
   bits).

 - Since IEEE double exp 0 is 0x3ff, probobly, for tilegx internally,
   0x3ff shoul be '0', too (don't move mantissa).

 - After analyzing the data from floatunssidf2 in gcc tilegx.md (the
   table above is part for it), I found that "(0x21b00 << 1) - 0x37 =
   0x3ff" is the best to match "all things".

So, I guess: the double exp is from bit 7 to bit 17, then the mantissa
is 60 bits (55 + 1 + 4), it from bit 0 to bit 59.

We can use 7 - 19 bits for normal exp calculation, then can get the real
exp (7 - 17 bits) with the overflow and underflow (so, I guess 18 bit is
for overflow, and 19 bit is for underflow).

>> +#if 0
>> +uint64_t exp : 11;/* exp, 0x21b << 1: 55 + 
>> TILEGX_F_EXP_DZERO */
>> +uint64_t ov : 1;  /* overflow for mul, low priority */
>> +uint64_t uv : 1;  /* underflow for mul, high priority */
>> +#endif
> 
> 
> No if 0.
> 

OK, I shall remove them.

>> +#pragma pack(pop)
> 
> Huh?  What are you attempting to do here?
> 

It is for matching "#pragma pack(push, 1)" which is above all related
struct/unions in this header file.

For the bit fields, we use uint64_t, but gcc still treate it as two
uint32_t, so for safety reason, I use pragma pack for our structures.

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH 2/4] target-tilegx: Implement fpu single floating point

2015-11-12 Thread Chen Gang
On 11/13/15 00:18, Richard Henderson wrote:
> On 11/12/2015 05:12 PM, Chen Gang wrote:
>> On 11/12/15 22:36, Richard Henderson wrote:
>>>> +if (sfmt.calc == TILEGX_F_CALC_CVT) {
>>>> +if (sfmt.sign)
>>>> +f.f = int32_to_float32(0 - sfmt.mantissa, fp_status);
>>>> +else
>>>> +f.f = uint32_to_float32(sfmt.mantissa, fp_status);
>>>> +} else {
>>>
>>> Formatting.
>>>
>>> You really should know better by now.
>>> I'm not even going to look at the rest.
>>>
>>
>> Excuse me, my English is not quite well, I am not quite understand your
>> meaning.
>>
>> Does the code above have issues?
>>
> 
> Please read ./CODING_STYLE, and use ./scripts/checkpatch.pl.
> 

Oh, sorry.



Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH 2/4] target-tilegx: Implement fpu single floating point

2015-11-12 Thread Chen Gang
On 11/12/15 22:36, Richard Henderson wrote:
>> +if (sfmt.calc == TILEGX_F_CALC_CVT) {
>> +if (sfmt.sign)
>> +f.f = int32_to_float32(0 - sfmt.mantissa, fp_status);
>> +else
>> +f.f = uint32_to_float32(sfmt.mantissa, fp_status);
>> +} else {
> 
> Formatting.
> 
> You really should know better by now.
> I'm not even going to look at the rest.
> 

Excuse me, my English is not quite well, I am not quite understand your
meaning.

Does the code above have issues?

Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file

2015-11-12 Thread Chen Gang
On 11/13/15 00:10, Peter Maydell wrote:
> On 12 November 2015 at 16:04, Chen Gang <xili_gchen_5...@hotmail.com> wrote:
>> On 11/12/15 22:34, Richard Henderson wrote:
>>> On 11/08/2015 06:43 AM, Chen Gang wrote:
>>>
>>>> +#if !defined(HOST_WORDS_BIGENDIAN)
>>>> +/* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md 
>>>> */
>>>> +uint64_t exp : 8; /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */
>>>> +uint64_t uiknown0 : 2;/* unknown */
>>>
>>> I would really rather you didn't use bitfields, because of exactly this 
>>> sort of endianness problem.  Because, really, you can't trust this layout.  
>>> But I won't press this point, because it is complicated enough already.
>>>
>>
>> Because of endianess issues, for me, I don't like bit fields either. But
>> I can not find any other simpler ways than current.
> 
>> OK, I shall remove them.
>>
>>>> +#pragma pack(pop)
>>>
>>> Huh?  What are you attempting to do here?
>>>
>>
>> It is for matching "#pragma pack(push, 1)" which is above all related
>> struct/unions in this header file.
> 
> Please don't use 'pragma pack' or bitfields. If you need to pack
> and unpack things from a target-CPU defined field use bit operations
> and/or extract32/deposit32/extract64/deposit64.
> 

OK, thanks, although it means I have to rewrite most of code, and test
them again. I shall try to send patch v2 within the next week.

Thanks.
-- 
Chen Gang (陈刚)

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



[Qemu-devel] [PATCH 3/4] target-tilegx: Implement fpu fdouble floating point

2015-11-07 Thread Chen Gang
>From c467db1c0a5f4c6560b8b2115732aa718c4b Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Sun, 8 Nov 2015 09:10:17 +0800
Subject: [PATCH 3/4] target-tilegx: Implement fpu fdouble floating point
 instructions

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/fdouble_helper.c | 290 +
 1 file changed, 290 insertions(+)
 create mode 100644 target-tilegx/fdouble_helper.c

diff --git a/target-tilegx/fdouble_helper.c b/target-tilegx/fdouble_helper.c
new file mode 100644
index 000..7ba0583
--- /dev/null
+++ b/target-tilegx/fdouble_helper.c
@@ -0,0 +1,290 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "fpu.h"
+
+#define TILEGX_F_MAN_HBIT        (1ULL << 59)
+
+#pragma pack(push, 1)
+typedef union F64Fmt {
+    float64 d;
+    struct {
+#if defined(HOST_WORDS_BIGENDIAN)
+        uint64_t sign : 1;
+        uint64_t exp  : 11;
+        uint64_t frac : 52;
+#else
+        uint64_t frac : 52;
+        uint64_t exp  : 11;
+        uint64_t sign : 1;
+#endif
+    } bits;
+} F64Fmt;
+#pragma pack(pop)
+
+static uint64_t fr_to_man(F64Fmt v)
+{
+    uint64_t val = (uint64_t)v.bits.frac << 7;
+
+    if (v.bits.exp)
+        val |= TILEGX_F_MAN_HBIT;
+
+    return val;
+}
+
+uint64_t helper_fdouble_unpack_min(CPUTLGState *env,
+                                   uint64_t srca, uint64_t srcb)
+{
+    F64Fmt va, vb;
+    TileGXFPDFmtV v;
+
+    va.d = make_float64(srca);
+    vb.d = make_float64(srcb);
+    v.ll = 0; /* also cause v.fmt.overflow = 0 */
+
+    if (float64_is_any_nan(srca) || float64_is_any_nan(srcb)
+        || float64_is_infinity(srca) || float64_is_infinity(srcb))
+        ;
+    else if (va.bits.exp> vb.bits.exp) {
+        if (va.bits.exp - vb.bits.exp < 64)
+            v.fmt.mantissa = fr_to_man(vb)>> (va.bits.exp - vb.bits.exp);
+    } else if (va.bits.exp < vb.bits.exp) {
+        if (vb.bits.exp - va.bits.exp < 64)
+            v.fmt.mantissa = fr_to_man(va)>> (vb.bits.exp - va.bits.exp);
+    } else if (va.bits.frac> vb.bits.frac)
+        v.fmt.mantissa = fr_to_man(vb);
+    else
+        v.fmt.mantissa = fr_to_man(va);
+
+    return v.ll;
+}
+
+uint64_t helper_fdouble_unpack_max(CPUTLGState *env,
+                                   uint64_t srca, uint64_t srcb)
+{
+    F64Fmt va, vb;
+    TileGXFPDFmtV v;
+
+    va.d = make_float64(srca);
+    vb.d = make_float64(srcb);
+    v.ll = 0; /* also cause v.fmt.overflow = 0 */
+
+    if (float64_is_any_nan(srca) || float64_is_any_nan(srcb)
+        || float64_is_infinity(srca) || float64_is_infinity(srcb))
+        ;
+    else if (va.bits.exp> vb.bits.exp)
+        v.fmt.mantissa = fr_to_man(va);
+    else if (va.bits.exp < vb.bits.exp)
+        v.fmt.mantissa = fr_to_man(vb);
+    else if (va.bits.frac> vb.bits.frac)
+        v.fmt.mantissa = fr_to_man(va);
+    else
+        v.fmt.mantissa = fr_to_man(vb);
+
+    return v.ll;
+}
+
+uint64_t helper_fdouble_addsub(CPUTLGState *env,
+                               uint64_t dest, uint64_t srca, uint64_t srcb)
+{
+    TileGXFPDFmtF flags;
+    TileGXFPDFmtV v;
+
+    flags.ll = srcb;
+    if (flags.fmt.calc == TILEGX_F_CALC_ADD) {
+        v.ll = dest + srca; /* maybe set addsub overflow bit */
+    } else
+        v.ll = dest - srca;
+
+    return v.ll;
+}
+
+/* absolute-add/mul may cause add/mul carry or overflow */
+static bool proc_oflow(TileGXFPDFmtF *flags, TileGXFPDFmtV *v, uint64_t *srcb)
+{
+    if (v->fmt.overflow) {
+        flags->fmt.vexp++;
+        *srcb>>= 1;
+        *srcb |= (uint64_t)v->ll << 63;
+        v->ll>>= 1;
+        v->fmt.overflow = 0;
+    }
+    return flags->fmt.vexp> TILEGX_F_EXP_DMAX;
+}
+
+uint64_t helper_fdouble_pack2(CPUTLGState *env,
+                              uint64_t dest, uint64_t srca, uint64_t srcb)
+{
+    TileGXFPDFmtF flags;
+    TileGXFPDFmtV v;
+    F64Fmt d;
+
+    flags.ll = dest;
+    v.ll = srca;
+
+    d.d = 0;
+    d.bits.sign = flags.fmt.sign;
+
+    /*
+     * fdouble

[Qemu-devel] [PATCH 4/4] target-tilegx: Let fpu implementation code can be built and used

2015-11-07 Thread Chen Gang
>From 8fab455a5ac5508d06cc69f778e926ad098fbe5b Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Sun, 8 Nov 2015 09:11:36 +0800
Subject: [PATCH 4/4] target-tilegx: Let fpu implementation code can be built
 and used

It passes gcc testsuite: it can get the same result as the original fpu
implementation have done.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/Makefile.objs |  3 +-
 target-tilegx/cpu.h         |  2 ++
 target-tilegx/helper.h      | 12 
 target-tilegx/translate.c   | 68 +++--
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
index 0db778f..c2cf2f1 100644
--- a/target-tilegx/Makefile.objs
+++ b/target-tilegx/Makefile.objs
@@ -1 +1,2 @@
-obj-y += cpu.o translate.o helper.o simd_helper.o
+obj-y += cpu.o translate.o helper.o simd_helper.o \
+   fsingle_helper.o fdouble_helper.o
diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
index 03df107..445a606 100644
--- a/target-tilegx/cpu.h
+++ b/target-tilegx/cpu.h
@@ -88,6 +88,8 @@ typedef struct CPUTLGState {
     uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */
     uint64_t pc;                       /* Current pc */
 
+    float_status fp_status;            /* floating point status */
+
 #if defined(CONFIG_USER_ONLY)
     uint64_t excaddr;                  /* exception address */
     uint64_t atomic_srca;              /* Arguments to atomic "exceptions" */
diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
index 9281d0f..b785bf2 100644
--- a/target-tilegx/helper.h
+++ b/target-tilegx/helper.h
@@ -24,3 +24,15 @@ DEF_HELPER_FLAGS_2(v1shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shl, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shru, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(v2shrs, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+
+DEF_HELPER_3(fsingle_add1, i64, env, i64, i64)
+DEF_HELPER_3(fsingle_sub1, i64, env, i64, i64)
+DEF_HELPER_3(fsingle_mul1, i64, env, i64, i64)
+DEF_HELPER_2(fsingle_pack2, i64, env, i64)
+DEF_HELPER_3(fdouble_unpack_min, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_unpack_max, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_add_flags, i64, env, i64, i64)
+DEF_HELPER_3(fdouble_sub_flags, i64, env, i64, i64)
+DEF_HELPER_4(fdouble_addsub, i64, env, i64, i64, i64)
+DEF_HELPER_3(fdouble_mul_flags, i64, env, i64, i64)
+DEF_HELPER_4(fdouble_pack2, i64, env, i64, i64, i64)
diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index b8ca401..4d74b1d 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -597,6 +597,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
         }
         qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s", mnemonic, reg_names[srca]);
         return ret;
+
+    case OE_RR_X0(FSINGLE_PACK1):
+    case OE_RR_Y0(FSINGLE_PACK1):
+        mnemonic = "fsingle_pack1";
+        goto done2;
     }
 
     tdest = dest_gr(dc, dest);
@@ -613,9 +618,6 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
         gen_helper_cnttz(tdest, tsrca);
         mnemonic = "cnttz";
         break;
-    case OE_RR_X0(FSINGLE_PACK1):
-    case OE_RR_Y0(FSINGLE_PACK1):
-        return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
     case OE_RR_X1(LD1S):
         memop = MO_SB;
         mnemonic = "ld1s"; /* prefetch_l1_fault */
@@ -734,6 +736,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
         return TILEGX_EXCP_OPCODE_UNKNOWN;
     }
 
+done2:
     qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s %s, %s", mnemonic,
                   reg_names[dest], reg_names[srca]);
     return ret;
@@ -742,13 +745,21 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned 
opext,
 static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
                                unsigned dest, unsigned srca, unsigned srcb)
 {
-    TCGv tdest = dest_gr(dc, dest);
-    TCGv tsrca = load_gr(dc, srca);
-    TCGv tsrcb = load_gr(dc, srcb);
+    TCGv tdest, tsrca, tsrcb;
     TCGv t0;
     const char *mnemonic;
 
     switch (opext) {
+    case OE_RRR(FSINGLE_ADDSUB2, 0, X0):
+        mnemonic = "fsingle_addsub2";
+        goto done2;
+    }
+
+    tdest = dest_gr(dc, dest);
+    tsrca = load_gr(dc, srca);
+    tsrcb = load_gr(dc, srcb);
+
+    switch (opext) {
     case OE_RRR(ADDXSC, 0, X0):
     case OE_RRR(ADDXSC, 0, X1):
         gen_saturate_op(tdest, tsrca, tsrcb, tcg_gen_add_tl);
@@ -906,14 +917,39 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned 
opext,
         mnemonic = "exch";
         break;
     case OE_RRR(FDOUBLE_ADDSUB, 0, X0):
+        gen_helper_fdouble_addsub(tdest, cpu_env,
+                                  load_gr(dc, dest),tsrca, tsrcb);
+        mnemonic = "fdouble_addsub";
+        break;
     

[Qemu-devel] [PATCH 1/4] target-tilegx: Add fpu header file

2015-11-07 Thread Chen Gang
>From 91a3d7d591cac6c4a39d4dbc5c3ffe8c17b10b6a Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Sun, 8 Nov 2015 09:05:29 +0800
Subject: [PATCH 1/4] target-tilegx: Add fpu header file

It defines the main data structures for tilegx fpu. Also it provides the
summary description for each floating point instructions impmementation.

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/fpu.h | 228 
 1 file changed, 228 insertions(+)
 create mode 100644 target-tilegx/fpu.h

diff --git a/target-tilegx/fpu.h b/target-tilegx/fpu.h
new file mode 100644
index 000..adfa1f5
--- /dev/null
+++ b/target-tilegx/fpu.h
@@ -0,0 +1,228 @@
+/*
+ *  TILE-Gx virtual FPU header
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef FPU_TILEGX_H
+#define FPU_TILEGX_H
+
+/*
+ * From IEEE standard, exp of float is 8-bits, exp of double is 11-bits.
+ */
+#define TILEGX_F_EXP_FZERO  0x7f  /* Zero exp for single 8-bits */
+#define TILEGX_F_EXP_FMAX   0xfe  /* max exp for single 8-bits */
+#define TILEGX_F_EXP_DZERO  0x3ff /* Zero exp for double 11-bits */
+#define TILEGX_F_EXP_DMAX   0x7fe /* max exp for double 11-bits */
+#define TILEGX_F_EXP_DUF    0x1000/* underflow exp bit for double */
+
+/*
+ * For fdouble absolute cacluation flag
+ */
+#define TILEGX_F_CALC_CVT   0     /* Perform int to fsingle/fdouble */
+#define TILEGX_F_CALC_ADD   1     /* Perform absolute add operation */
+#define TILEGX_F_CALC_SUB   2     /* Perform absolute sub operation */
+#define TILEGX_F_CALC_MUL   3     /* Perform absolute mul operation */
+
+#pragma pack(push, 1)
+
+/*
+ * Single format, it is 64-bit.
+ *
+ * Single exp analyzing: 0x9e - 0x1e(30) = 0x80
+ *
+ *   7   6   5   4   3   2   1   0
+ *
+ *   1   0   0   1   1   1   1   0
+ *
+ *   0   0   0   1   1   1   1   1    => 0x1f(31)
+ *
+ *   0   1   1   1   1   1   1   1    => 0x7f
+ */
+typedef struct TileGXFPSFmt {
+
+#if !defined(HOST_WORDS_BIGENDIAN)
+    /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */
+    uint64_t exp : 8;             /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */
+    uint64_t uiknown0 : 2;        /* unknown */
+    uint64_t sign : 1;            /* Sign bit for the total value */
+    uint64_t calc: 2;             /* calculation flag */
+    uint64_t unknown1 : 12;       /* unknown */
+
+    /* Come from TILE-Gx ISA document, Table 7-2 for floating point */
+    uint64_t unordered : 1;       /* The two are unordered */
+    uint64_t lt : 1;              /* 1st is less than 2nd */
+    uint64_t le : 1;              /* 1st is less than or equal to 2nd */
+    uint64_t gt : 1;              /* 1st is greater than 2nd */
+    uint64_t ge : 1;              /* 1st is greater than or equal to 2nd */
+    uint64_t eq : 1;              /* The two operands are equal */
+    uint64_t neq : 1;             /* The two operands are not equal */
+
+    /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */
+    uint64_t mantissa : 32;       /* mantissa */
+#else
+    uint64_t mantissa : 32;       /* mantissa */
+    uint64_t neq : 1;             /* The two operands are not equal */
+    uint64_t eq : 1;              /* The two operands are equal */
+    uint64_t ge : 1;              /* 1st is greater than or equal to 2nd */
+    uint64_t gt : 1;              /* 1st is greater than 2nd */
+    uint64_t le : 1;              /* 1st is less than or equal to 2nd */
+    uint64_t lt : 1;              /* 1st is less than 2nd */
+    uint64_t unordered : 1;       /* The two are unordered */
+    uint64_t unknown1 : 12;       /* unknown */
+    uint64_t calc: 2;             /* calculation flag */
+    uint64_t sign : 1;            /* Sign bit for the total value */
+    uint64_t unknown0 : 2;        /* unknown */
+    uint64_t exp : 8;             /* exp, 0x9e: 31 + TILEGX_F_EXP_FZERO */
+#endif
+} TileGXFPSFmt;
+/*
+ * FSingle instructions implemenation:
+ *
+ * fsingle_add1         ; calc srca and srcb,
+ *                      ; convert float_32 to TileGXFPSFmt result.
+ *                      ; move TileGXFPSFmt result to dest.
+ *
+ * fsingle_sub1         ; calc srca and srcb.
+ *                      ; convert float_32 to TileGXFPSFmt result.
+ *                      ; move 

[Qemu-devel] [PATCH 2/4] target-tilegx: Implement fpu single floating point

2015-11-07 Thread Chen Gang
>From 3270485ebd56429f6f62f0a4f967009d8a1b14d6 Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Sun, 8 Nov 2015 09:08:40 +0800
Subject: [PATCH 2/4] target-tilegx: Implement fpu single floating point
 instructions

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/fsingle_helper.c | 168 +
 1 file changed, 168 insertions(+)
 create mode 100644 target-tilegx/fsingle_helper.c

diff --git a/target-tilegx/fsingle_helper.c b/target-tilegx/fsingle_helper.c
new file mode 100644
index 000..f8f6b69
--- /dev/null
+++ b/target-tilegx/fsingle_helper.c
@@ -0,0 +1,168 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "fpu.h"
+
+#pragma pack(push, 1)
+typedef union F32Fmt {
+    uint32_t f;
+    struct {
+#if defined(HOST_WORDS_BIGENDIAN)
+        uint32_t sign : 1;
+        uint32_t exp  : 8;
+        uint32_t frac : 23;
+#else
+        uint32_t frac : 23;
+        uint32_t exp  : 8;
+        uint32_t sign : 1;
+#endif
+    } bits;
+} F32Fmt;
+#pragma pack(pop)
+
+static uint64_t sfmt_to_uint64(TileGXFPSFmt a)
+{
+    union {
+        TileGXFPSFmt a;
+        uint64_t v;
+    } t;
+    t.a = a;
+    return t.v;
+}
+
+static TileGXFPSFmt uint64_to_sfmt(uint64 v)
+{
+    union {
+        TileGXFPSFmt a;
+        uint64_t v;
+    } t;
+    t.v = v;
+    return t.a;
+}
+
+static TileGXFPSFmt float32_to_sfmt(float32 f)
+{
+    F32Fmt tmp = {f};
+    union {
+        TileGXFPSFmt fmt;
+        uint64_t v;
+    } sfmt;
+
+    sfmt.v = 0;
+    sfmt.fmt.sign = tmp.bits.sign;
+    sfmt.fmt.exp = tmp.bits.exp;
+    sfmt.fmt.mantissa = (tmp.bits.frac << 8) | (1 << 31);
+
+    return sfmt.fmt;
+}
+
+static float32 sfmt_to_float32(TileGXFPSFmt sfmt, float_status *fp_status)
+{
+    F32Fmt f;
+
+    if (sfmt.calc == TILEGX_F_CALC_CVT) {
+        if (sfmt.sign)
+            f.f = int32_to_float32(0 - sfmt.mantissa, fp_status);
+        else
+            f.f = uint32_to_float32(sfmt.mantissa, fp_status);
+    } else {
+        f.bits.sign = sfmt.sign;
+        f.bits.exp = sfmt.exp;
+        f.bits.frac = sfmt.mantissa>> 8;
+    }
+
+    return f.f;
+}
+
+uint64_t helper_fsingle_pack2(CPUTLGState *env, uint64_t srca)
+{
+    return float32_val(sfmt_to_float32(uint64_to_sfmt(srca), >fp_status));
+}
+
+static void ana_bits(float_status *fp_status,
+                     float32 fsrca, float32 fsrcb, TileGXFPSFmt *sfmt)
+{
+    if (float32_eq(fsrca, fsrcb, fp_status)) {
+        sfmt->eq = 1;
+    } else {
+        sfmt->neq = 1;
+    }
+
+    if (float32_lt(fsrca, fsrcb, fp_status)) {
+        sfmt->lt = 1;
+    }
+    if (float32_le(fsrca, fsrcb, fp_status)) {
+        sfmt->le = 1;
+    }
+
+    if (float32_lt(fsrcb, fsrca, fp_status)) {
+        sfmt->gt = 1;
+    }
+    if (float32_le(fsrcb, fsrca, fp_status)) {
+        sfmt->ge = 1;
+    }
+
+    if (float32_unordered(fsrca, fsrcb, fp_status)) {
+        sfmt->unordered = 1;
+    }
+}
+
+static uint64_t main_calc(float_status *fp_status,
+                          float32 fsrca, float32 fsrcb,
+                          float32 (*calc)(float32, float32, float_status *))
+{
+    TileGXFPSFmt sfmt = float32_to_sfmt(calc(fsrca, fsrcb, fp_status));
+
+    ana_bits(fp_status, fsrca, fsrcb, );
+
+    if (calc == float32_add)
+        sfmt.calc = TILEGX_F_CALC_ADD;
+    else if (calc == float32_sub)
+        sfmt.calc = TILEGX_F_CALC_SUB;
+    else
+        sfmt.calc = TILEGX_F_CALC_MUL;
+
+    sfmt_to_float32(sfmt, fp_status);
+
+    return sfmt_to_uint64(sfmt);
+}
+
+uint64_t helper_fsingle_add1(CPUTLGState *env, uint64_t srca, uint64_t srcb)
+{
+    return main_calc(>fp_status,
+                     make_float32(srca), make_float32(srcb), float32_add);
+}
+
+uint64_t helper_fsingle_sub1(CPUTLGState *env, uint64_t srca, uint64_t srcb)
+{
+    return main_calc(>fp_status,
+                     make_float32(srca), make_float32(srcb), float32_sub);
+}
+
+uint64_t helper_fsingle_mul1(CPUTLGState *env, uint64_t srca, uint64_t srcb)
+{
+  

[Qemu-devel] [PATCH 0/4] Implment fpu floating point instructions

2015-11-07 Thread Chen Gang
>From 8fab455a5ac5508d06cc69f778e926ad098fbe5b Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Sun, 8 Nov 2015 09:28:16 +0800
Subject: [PATCH 0/4] Implment fpu floating point instructions

It passes gcc testsuite: get the same result like the original floating
point implementation has done (original implementation is temporary).

Chen Gang (4):
  target-tilegx: Add fpu header file
  target-tilegx: Implement fpu single floating point instructions
  target-tilegx: Implement fpu fdouble floating point instructions
  target-tilegx: Let fpu implementation code can be built and used

 target-tilegx/Makefile.objs    |   3 +-
 target-tilegx/cpu.h            |   2 +
 target-tilegx/fdouble_helper.c | 290 +
 target-tilegx/fpu.h            | 228 
 target-tilegx/fsingle_helper.c | 168 
 target-tilegx/helper.h         |  12 ++
 target-tilegx/translate.c      |  68 --
 7 files changed, 761 insertions(+), 10 deletions(-)
 create mode 100644 target-tilegx/fdouble_helper.c
 create mode 100644 target-tilegx/fpu.h
 create mode 100644 target-tilegx/fsingle_helper.c

-- 
1.9.3

  


Re: [Qemu-devel] [PATCH] target-tilegx: Implement floating point instructions

2015-11-01 Thread Chen Gang

Sorry, based on this patch, after fix 5 issues, it still has issues. The
issues which I have fixed are:

 - Fix floating point fdouble multiply related instructions issues

 - Process every overflow cases.

 - Process absolute multiply carrying just like absolute add has done.

 - Process the nearest skipped bit carrying for fdouble.

 - Process the nearest skipped bit carrying for fsingle.

I shall try to fix the left issues all within this week (2015-11-08).

Thanks.

On 11/1/15 12:30, Chen Gang wrote:
>
> Oh, sorry, it can not pass gcc testsuite: the fdouble mul insns have
> issues (original temporary implementation had no this cases -- it
> skipped the outside mul operation).
>
> After it passes gcc testtsuite, I shall split it into several small
> patches, and send patch v2 (I shall try to finish today).
>
> Thanks.
>
>
> On 11/1/15 00:59, Chen Gang wrote:
>> From 42733d085bfcb4882cfa4eb25a9387e3d953a64f Mon Sep 17 00:00:00 2001
>> From: Chen Gang <gang.chen.5...@gmail.com>
>> Date: Sun, 1 Nov 2015 00:50:33 +0800
>> Subject: [PATCH] target-tilegx: Implement floating point instructions
>>
>> It is implenented in a normal way, and passed unit tests (8 test cases).
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>> target-tilegx/Makefile.objs | 3 +-
>> target-tilegx/cpu.h | 2 +
>> target-tilegx/fdouble_helper.c | 234 
>> +
>> target-tilegx/fpu.h | 217 ++
>> target-tilegx/fsingle_helper.c | 157 +++
>> target-tilegx/helper.h | 12 +++
>> target-tilegx/translate.c | 68 ++--
>> 7 files changed, 683 insertions(+), 10 deletions(-)
>> create mode 100644 target-tilegx/fdouble_helper.c
>> create mode 100644 target-tilegx/fpu.h
>> create mode 100644 target-tilegx/fsingle_helper.c
>>
>> diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
>> index 0db778f..c2cf2f1 100644
>> --- a/target-tilegx/Makefile.objs
>> +++ b/target-tilegx/Makefile.objs
>> @@ -1 +1,2 @@
>> -obj-y += cpu.o translate.o helper.o simd_helper.o
>> +obj-y += cpu.o translate.o helper.o simd_helper.o \
>> + fsingle_helper.o fdouble_helper.o
>> diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
>> index 03df107..445a606 100644
>> --- a/target-tilegx/cpu.h
>> +++ b/target-tilegx/cpu.h
>> @@ -88,6 +88,8 @@ typedef struct CPUTLGState {
>> uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */
>> uint64_t pc; /* Current pc */
>>
>> + float_status fp_status; /* floating point status */
>> +
>> #if defined(CONFIG_USER_ONLY)
>> uint64_t excaddr; /* exception address */
>> uint64_t atomic_srca; /* Arguments to atomic "exceptions" */
>> diff --git a/target-tilegx/fdouble_helper.c b/target-tilegx/fdouble_helper.c
>> new file mode 100644
>> index 000..b3b0588
>> --- /dev/null
>> +++ b/target-tilegx/fdouble_helper.c
>> @@ -0,0 +1,234 @@
>> +/*
>> + * QEMU TILE-Gx helpers
>> + *
>> + * Copyright (c) 2015 Chen Gang
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see
>> + * <http://www.gnu.org/licenses/lgpl-2.1.html>
>> + */
>> +
>> +#include "cpu.h"
>> +#include "qemu-common.h"
>> +#include "exec/helper-proto.h"
>> +#include "fpu/softfloat.h"
>> +
>> +#include "fpu.h"
>> +
>> +#define TILEGX_F_MAN_HBIT (1ULL << 59)
>> +
>> +#pragma pack(push, 1)
>> +typedef union F64Fmt {
>> + float64 d;
>> + struct {
>> +#if defined(HOST_WORDS_BIGENDIAN)
>> + uint64_t sign : 1;
>> + uint64_t exp : 11;
>> + uint64_t frac : 52;
>> +#else
>> + uint64_t frac : 52;
>> + uint64_t exp : 11;
>> + uint64_t sign : 1;
>> +#endif
>> + } bits;
>> +} F64Fmt;
>> +#pragma pack(pop)
>> +
>> +static uint64_t fr_t

Re: [Qemu-devel] [PATCH] target-tilegx: Implement floating point instructions

2015-10-31 Thread Chen Gang

Oh, sorry, it can not pass gcc testsuite: the fdouble mul insns have
issues (original temporary implementation had no this cases -- it
skipped the outside mul operation).

After it passes gcc testtsuite, I shall split it into several small
patches, and send patch v2 (I shall try to finish today).

Thanks.


On 11/1/15 00:59, Chen Gang wrote:
> From 42733d085bfcb4882cfa4eb25a9387e3d953a64f Mon Sep 17 00:00:00 2001
> From: Chen Gang <gang.chen.5...@gmail.com>
> Date: Sun, 1 Nov 2015 00:50:33 +0800
> Subject: [PATCH] target-tilegx: Implement floating point instructions
> 
> It is implenented in a normal way, and passed unit tests (8 test cases).
> 
> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
> ---
>  target-tilegx/Makefile.objs|   3 +-
>  target-tilegx/cpu.h|   2 +
>  target-tilegx/fdouble_helper.c | 234 
> +
>  target-tilegx/fpu.h| 217 ++
>  target-tilegx/fsingle_helper.c | 157 +++
>  target-tilegx/helper.h |  12 +++
>  target-tilegx/translate.c  |  68 ++--
>  7 files changed, 683 insertions(+), 10 deletions(-)
>  create mode 100644 target-tilegx/fdouble_helper.c
>  create mode 100644 target-tilegx/fpu.h
>  create mode 100644 target-tilegx/fsingle_helper.c
> 
> diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
> index 0db778f..c2cf2f1 100644
> --- a/target-tilegx/Makefile.objs
> +++ b/target-tilegx/Makefile.objs
> @@ -1 +1,2 @@
> -obj-y += cpu.o translate.o helper.o simd_helper.o
> +obj-y += cpu.o translate.o helper.o simd_helper.o \
> + fsingle_helper.o fdouble_helper.o
> diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
> index 03df107..445a606 100644
> --- a/target-tilegx/cpu.h
> +++ b/target-tilegx/cpu.h
> @@ -88,6 +88,8 @@ typedef struct CPUTLGState {
>  uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside 
> */
>  uint64_t pc;   /* Current pc */
>  
> +float_status fp_status;/* floating point status */
> +
>  #if defined(CONFIG_USER_ONLY)
>  uint64_t excaddr;  /* exception address */
>  uint64_t atomic_srca;  /* Arguments to atomic "exceptions" */
> diff --git a/target-tilegx/fdouble_helper.c b/target-tilegx/fdouble_helper.c
> new file mode 100644
> index 000..b3b0588
> --- /dev/null
> +++ b/target-tilegx/fdouble_helper.c
> @@ -0,0 +1,234 @@
> +/*
> + * QEMU TILE-Gx helpers
> + *
> + *  Copyright (c) 2015 Chen Gang
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see
> + * <http://www.gnu.org/licenses/lgpl-2.1.html>
> + */
> +
> +#include "cpu.h"
> +#include "qemu-common.h"
> +#include "exec/helper-proto.h"
> +#include "fpu/softfloat.h"
> +
> +#include "fpu.h"
> +
> +#define TILEGX_F_MAN_HBIT(1ULL << 59)
> +
> +#pragma pack(push, 1)
> +typedef union F64Fmt {
> +float64 d;
> +struct {
> +#if defined(HOST_WORDS_BIGENDIAN)
> +uint64_t sign : 1;
> +uint64_t exp  : 11;
> +uint64_t frac : 52;
> +#else
> +uint64_t frac : 52;
> +uint64_t exp  : 11;
> +uint64_t sign : 1;
> +#endif
> +} bits;
> +} F64Fmt;
> +#pragma pack(pop)
> +
> +static uint64_t fr_to_man(F64Fmt v)
> +{
> +uint64_t val = (uint64_t)v.bits.frac << 7;
> +
> +if (v.bits.exp)
> +val |= TILEGX_F_MAN_HBIT;
> +
> +return val;
> +}
> +
> +uint64_t helper_fdouble_unpack_min(CPUTLGState *env,
> +   uint64_t srca, uint64_t srcb)
> +{
> +F64Fmt va, vb;
> +TileGXFPDFmtV v;
> +
> +va.d = make_float64(srca);
> +vb.d = make_float64(srcb);
> +v.ll = 0; /* also cause v.fmt.overflow = 0 */
> +
> +if (va.bits.exp> vb.bits.exp)
> +v.fmt.mantissa = fr_to_man(vb)>> (va.bits.exp - vb.bits.exp);
> +else if (va.bits.exp < vb.bits.exp)
> +   

Re: [Qemu-devel] [PULL] tilegx queued patch

2015-10-31 Thread Chen Gang
On 10/31/15 04:44, Richard Henderson wrote:
> Just one mergable patch since the last update.
> I guess this will be it before hard-freeze.
>

Thank you for your work.

I have finished coding for the implementation of tilegx floating point
instructions, at present, I am just testing and fixing the related
issues.

Hope I can finish today and send patches for it (although I am not quite
sure).


Thanks.
--
Chen Gang (陈刚)

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

[Qemu-devel] [PATCH] target-tilegx: Implement floating point instructions

2015-10-31 Thread Chen Gang
>From 42733d085bfcb4882cfa4eb25a9387e3d953a64f Mon Sep 17 00:00:00 2001
From: Chen Gang <gang.chen.5...@gmail.com>
Date: Sun, 1 Nov 2015 00:50:33 +0800
Subject: [PATCH] target-tilegx: Implement floating point instructions

It is implenented in a normal way, and passed unit tests (8 test cases).

Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
---
 target-tilegx/Makefile.objs    |   3 +-
 target-tilegx/cpu.h            |   2 +
 target-tilegx/fdouble_helper.c | 234 +
 target-tilegx/fpu.h            | 217 ++
 target-tilegx/fsingle_helper.c | 157 +++
 target-tilegx/helper.h         |  12 +++
 target-tilegx/translate.c      |  68 ++--
 7 files changed, 683 insertions(+), 10 deletions(-)
 create mode 100644 target-tilegx/fdouble_helper.c
 create mode 100644 target-tilegx/fpu.h
 create mode 100644 target-tilegx/fsingle_helper.c

diff --git a/target-tilegx/Makefile.objs b/target-tilegx/Makefile.objs
index 0db778f..c2cf2f1 100644
--- a/target-tilegx/Makefile.objs
+++ b/target-tilegx/Makefile.objs
@@ -1 +1,2 @@
-obj-y += cpu.o translate.o helper.o simd_helper.o
+obj-y += cpu.o translate.o helper.o simd_helper.o \
+   fsingle_helper.o fdouble_helper.o
diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
index 03df107..445a606 100644
--- a/target-tilegx/cpu.h
+++ b/target-tilegx/cpu.h
@@ -88,6 +88,8 @@ typedef struct CPUTLGState {
     uint64_t spregs[TILEGX_SPR_COUNT]; /* Special used registers by outside */
     uint64_t pc;                       /* Current pc */
 
+    float_status fp_status;            /* floating point status */
+
 #if defined(CONFIG_USER_ONLY)
     uint64_t excaddr;                  /* exception address */
     uint64_t atomic_srca;              /* Arguments to atomic "exceptions" */
diff --git a/target-tilegx/fdouble_helper.c b/target-tilegx/fdouble_helper.c
new file mode 100644
index 000..b3b0588
--- /dev/null
+++ b/target-tilegx/fdouble_helper.c
@@ -0,0 +1,234 @@
+/*
+ * QEMU TILE-Gx helpers
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>
+ */
+
+#include "cpu.h"
+#include "qemu-common.h"
+#include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
+
+#include "fpu.h"
+
+#define TILEGX_F_MAN_HBIT        (1ULL << 59)
+
+#pragma pack(push, 1)
+typedef union F64Fmt {
+    float64 d;
+    struct {
+#if defined(HOST_WORDS_BIGENDIAN)
+        uint64_t sign : 1;
+        uint64_t exp  : 11;
+        uint64_t frac : 52;
+#else
+        uint64_t frac : 52;
+        uint64_t exp  : 11;
+        uint64_t sign : 1;
+#endif
+    } bits;
+} F64Fmt;
+#pragma pack(pop)
+
+static uint64_t fr_to_man(F64Fmt v)
+{
+    uint64_t val = (uint64_t)v.bits.frac << 7;
+
+    if (v.bits.exp)
+        val |= TILEGX_F_MAN_HBIT;
+
+    return val;
+}
+
+uint64_t helper_fdouble_unpack_min(CPUTLGState *env,
+                                   uint64_t srca, uint64_t srcb)
+{
+    F64Fmt va, vb;
+    TileGXFPDFmtV v;
+
+    va.d = make_float64(srca);
+    vb.d = make_float64(srcb);
+    v.ll = 0; /* also cause v.fmt.overflow = 0 */
+
+    if (va.bits.exp> vb.bits.exp)
+        v.fmt.mantissa = fr_to_man(vb)>> (va.bits.exp - vb.bits.exp);
+    else if (va.bits.exp < vb.bits.exp)
+        v.fmt.mantissa = fr_to_man(va)>> (vb.bits.exp - va.bits.exp);
+    else if (va.bits.frac> vb.bits.frac)
+        v.fmt.mantissa = fr_to_man(vb);
+    else
+        v.fmt.mantissa = fr_to_man(va);
+
+    return v.ll;
+}
+
+uint64_t helper_fdouble_unpack_max(CPUTLGState *env,
+                                   uint64_t srca, uint64_t srcb)
+{
+    F64Fmt va, vb;
+    TileGXFPDFmtV v;
+
+    va.d = make_float64(srca);
+    vb.d = make_float64(srcb);
+    v.ll = 0; /* also cause v.fmt.overflow = 0 */
+
+    if (va.bits.exp> vb.bits.exp)
+        v.fmt.mantissa = fr_to_man(va);
+    else if (va.bits.exp < vb.bits.exp)
+        v.fmt.mantissa = fr_to_man(vb);
+    else if (va.bits.frac> vb.bits.frac)
+        v.fmt.mantissa = fr_to_man(va);
+    else
+        v.fmt.mantissa = fr_to_man(vb);
+
+    return v.ll;
+}
+
+uint64_t helper_fdouble_addsub(CPUTLGState *env,
+                               uint64_t dest, uint64_

Re: [Qemu-devel] [Consult] tilegx: About floating point instructions

2015-10-28 Thread Chen Gang
gs    ; srca and srcb are float_64 value.
 *                      ; calc exp (exp_min), sign, and comp bits for flags.
 *                      ; set addsub bit to flags and move flags to dest.
 *
 * fdouble_sub_flags    ; srca and srcb are float_64 value.
 *                      ; calc exp (exp_min), sign, and comp bits for flags.
 *                      ; set addsub bit to flags and move flags to dest.
 *
 * fdouble_addsub:      ; dest, srca (max, min mantissa), and srcb (flags).
 *                      ; "dest +/- srca" depend on the add/sub bit of flags.
 *                      ; move result mantissa to dest.
 *
 * fdouble_mul_flags:   ; srca and srcb are float_64 value.
 *                      ; calc sign (xor), exp (exp_min + exp_max), and comp 
bits.
 *                      ; mix sign, exp, and comp bits as flags to dest.
 *
 * fdouble_pack1        ; move srcb (flags) to dest.
 *
 * fdouble_pack2        ; srca, srcb (high, low mantissa), and dest (flags)
 *                      ; normalize and pack result from srca, srcb, and dest.
 *                      ; move result to dest.
 */

#pragma pack(pop)


Thanks.
-- 
Chen Gang (陈刚)

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

  

Re: [Qemu-devel] [Consult] tilegx: About floating point instructions

2015-10-26 Thread Chen Gang

I guess, for sign number, the highest bit will not be used, but for
unsigned number, the highest bit will be used (then can let sign and
unsigned number can use the same format contents).

On 10/25/15 23:38, Chen Gang wrote:
> 
> /*
>  * Single format, it is 64-bit.
>  */
> typedef struct TileGXFPSFmt {
> 
> /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */
> uint64_t exp : 8; /* exp, 0x9e: 30 + TILEGX_F_EXP_FZERO */
> uint64_t unknown0 : 1;/* unknown */
> uint64_t sign : 1;/* Sign bit for the total value */
> uint64_t unknown1 : 15;   /* unknown */
> 
> /* Come from TILE-Gx ISA document, Table 7-2 for floating point */
> uint64_t unordered : 1;   /* The two are unordered */
> uint64_t lt : 1;  /* 1st is less than 2nd */
> uint64_t le : 1;  /* 1st is less than or equal to 2nd */
> uint64_t gt : 1;  /* 1st is greater than 2nd */
> uint64_t ge : 1;  /* 1st is greater than or equal to 2nd */
> uint64_t eq : 1;  /* The two operands are equal */
> uint64_t neq : 1; /* The two operands are not equal */
> 
> /* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */
> uint64_t mantissa : 31;   /* mantissa */
> uint64_t unknown2 : 1;/* unknown */

It is not unknown2, it means 0x8000 when sign is 0, and meaningless
when sign is 1.


> } TileGXFPSFmt;
>

[...]
 
> 
> /*
>  * Dobule format, value, 64-bit.
>  */
> typedef struct TileGXFPDFmtV {
> uint64_t unknown0 : 4;/* unknown */
> uint64_t mantissa : 55;   /* mantissa */
> uint64_t unknown1 : 5;/* unknown */

I guess, unknow1 is 4 bits, and the other 1 bit is for (1ULL << 55) when
sign is 0, and meaningless when sign is 1.


> } TileGXFPDFmtV;
> 
> #pragma pack(pop)
> 
> 


Thanks.
-- 
Chen Gang (陈刚)

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



Re: [Qemu-devel] [Consult] tilegx: About floating point instructions

2015-10-25 Thread Chen Gang
On 8/18/15 01:31, Richard Henderson wrote:
> 
> For single-precision it appears that the format is
> 
>   63  31  24   10  9 0
>   [ mantissa with implicit and guard bits | cmp flags | ?? | s | exp ]
> 
> We are able to deduce the bias for the exponent based on the input gcc gives 
> us
> for floatunssisf: 0x9e == 2**31 when the mantissa is normalized.
>

For me, 0x9e == 2**30:

 - For int32_t: 31-bits for values, highest bit for sign. Mantissa can
   'move' 30 bits to express 31-bits values, and 31st bit will never
   'move'.

 - 0x9e - 0x1e(30) = 0x80, which is the smallest 8-bits number, in our
   case, 0x80 means: "do not 'move' mantissa bits".

 - According to IEEE standard, the exp of float type is 8-bits, so 0x80
   is really the smallest value of exp.

/*
 * Single exp analyzing: 0x9e - 0x1e(30) = 0x80
 *
 *   7   6   5   4   3   2   1   0
 *
 *   1   0   0   1   1   1   1   0
 *
 *   0   0   0   1   1   1   1   0=> 0x1e(30)
 *
 *   1   0   0   0   0   0   0   0=> 0x80
 */

[...]
 
> 
> For double-precision things are more complicated.  Precisely because there is
> no dedicated fdouble_mul[1-4] instructions, but instead gcc is to use a normal
> 128-bit integer multiplication on the mantissa.
> 
> For double-precision it appears that the format is
> 
>  63   57   40
>   unpack [ overflow bits? | mantissa with implicit bit | guard bits ]
> 
>  63   31  24   20  1980
>   flags  [ ?? | cmp flags | ?? | s | exp | ?? ]
> 
> Similarly we can compute the bias for exp as 0x21b == 2**53.
> Or is it 20 bits of exponent and 0x21b00 == 2**53?
> 

For me, exp is (0x21b << 1):

 - According to IEEE standard for double, it's exp is 11 bits, the
   smallest exp is 0x400.

 - So for 0x21b00, we can only consider about bits between 0 and 17 (the
   highest bit must be 1).

 - And it is 11 bits, so the real value of 0x21b00 is (0x21b << 1). Then
   we know it 'move' 0x36(54) bits to get integer, then we know fdouble
   mantissa have 55-bits internally.

/*
 * Double exp analyzing: (0x21b << 1) - 0x36(54) = 0x400
 *
 *   17  16  15  14  13  12  11  10   9   8   76   5   4   3   2   1   0
 *
 *1   0   0   0   0   1   1   0   1   1   00   0   0   0   0   0   0
 *
 *0   0   0   0   0   1   1   0   1   1   0=> 0x36(54)
 *
 *1   0   0   0   0   0   0   0   0   0   0=> 0x400
 *
 */

So for me, the format in C header file is:

#pragma pack(push, 1)

/*
 * Single format, it is 64-bit.
 */
typedef struct TileGXFPSFmt {

/* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */
uint64_t exp : 8; /* exp, 0x9e: 30 + TILEGX_F_EXP_FZERO */
uint64_t unknown0 : 1;/* unknown */
uint64_t sign : 1;/* Sign bit for the total value */
uint64_t unknown1 : 15;   /* unknown */

/* Come from TILE-Gx ISA document, Table 7-2 for floating point */
uint64_t unordered : 1;   /* The two are unordered */
uint64_t lt : 1;  /* 1st is less than 2nd */
uint64_t le : 1;  /* 1st is less than or equal to 2nd */
uint64_t gt : 1;  /* 1st is greater than 2nd */
uint64_t ge : 1;  /* 1st is greater than or equal to 2nd */
uint64_t eq : 1;  /* The two operands are equal */
uint64_t neq : 1; /* The two operands are not equal */

/* According to float(uns)sisf2 and float(uns)sidf2 in gcc tilegx.md */
uint64_t mantissa : 31;   /* mantissa */
uint64_t unknown2 : 1;/* unknown */
} TileGXFPSFmt;

/*
 * Dobule format, flag, 64-bit.
 */
typedef struct TileGXFPDFmtF {

uint64_t unknown0 : 7;/* unknown */
uint64_t exp : 11;/* exp, 0x21b << 1: 54 + TILEGX_F_EXP_DZERO */
uint64_t unknown1 : 2;/* unknown */
uint64_t sign : 1;/* Sign bit for the total value */
uint64_t unknown2: 4; /* unknown */

/* Come from TILE-Gx ISA document, Table 7-2 for floating point */
uint64_t unordered : 1;   /* The two are unordered */
uint64_t lt : 1;  /* 1st is less than 2nd */
uint64_t le : 1;  /* 1st is less than or equal to 2nd */
uint64_t gt : 1;  /* 1st is greater than 2nd */
uint64_t ge : 1;  /* 1st is greater than or equal to 2nd */
uint64_t eq : 1;  /* The two operands are equal */
uint64_t neq : 1; /* The two operands are not equal */

uint64_t unknown3 : 32;   /* unknown */
} TileGXFPDFmtF;

/*
 * Dobule format, value, 64-bit.
 */
typedef struct TileGXFPDFmtV {
uint64_t unknown0 : 4;/* unknown */
uint64_t mantissa : 55;   /* mantissa */
uint64_t unknown1 : 5;/* unknown */
} TileGXFPDFmtV;

#p

  1   2   3   4   5   6   7   >