From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38088) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1amjb6-00005y-SL for qemu-devel@nongnu.org; Sun, 03 Apr 2016 11:05:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1amjb5-0003NG-51 for qemu-devel@nongnu.org; Sun, 03 Apr 2016 11:05:52 -0400 MIME-Version: 1.0 from: "Aleksandar Markovic" message-id: <78c-57013100-1-639ee000@97078574> content-type: multipart/alternative; boundary="----=_=-_OpenGroupware_org_NGMime-1932-1459695918.227686-0------" date: Sun, 03 Apr 2016 17:05:18 +0200 in-reply-to: <56FEC6F0.9000803@imgtec.com> Subject: Re: [Qemu-devel] [PATCH 2/2] target-mips: Implement IEEE 754-2008 functionality for R6 and MSA instructions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leon Alrae Cc: peter.maydell@linaro.org, ehabkost@redhat.com, proljc@gmail.com, mark.cave-ayland@ilande.co.uk, qemu-devel@nongnu.org, kbastian@mail.uni-paderborn.de, agraf@suse.de, blauwirbel@gmail.com, jcmvbkbc@gmail.com, miodrag.dinic@imgtec.com, qemu-arm@nongnu.org, qemu-ppc@nongnu.org, petar.jovanovic@imgtec.com, edgar.iglesias@gmail.com, pbonzini@redhat.com, gxt@mprc.pku.edu.cn, afaerber@suse.de, aurelien@aurel32.net, rth@twiddle.net ------=_=-_OpenGroupware_org_NGMime-1932-1459695918.227686-0------ content-type: text/plain; charset=utf-8 content-length: 5105 content-transfer-encoding: quoted-printable Hello, Leon, thank you very much for the kind feedback. Let me clarify = my take on the involved issues. 1) Class operations I am going to correct the code as you hinted. The reason I wanted separate handling of MSA class operation is code an= d module decoupling. Handling of MSA instructions (in file msa=5Fhelper= .c) and regular instructions (in file op=5Fhelper.c) have many overlapi= ng areas - however, my understanding is that the designer of MSA module= wanted it to be as independant on code in other files/modulas as possi= ble. Handling class operation is on of the rare instances where code in= msa=5Fhelper.c relies on the code in op=5Fhelper.c., and it made sense= to me that this dependence should be removed, for the sake of consiste= ncy within MSA module - even if the functionalitied are virtually ident= ical. That said, I will anyway listen to your advice, since you most pr= obably see more than myself regarding this, and I am going to revert to= a single handling of class operations, for both MSA and regular versio= ns. 2) Flush subnormals My impression is that his set of features should be treated and impleme= nted separately, at some later point in time. Although the implementation seems not to be too complex (defining FCR31= =5FFS, invoking appropriately=C2=A0set=5Fflush=5Fto=5Fzero() and=C2=A0s= et=5Fflush=5Finputs=5Fto=5Fzero() on CPU init, plus special exception h= andling, like it is already done for MSA equivalents), it looks to me t= hat it would have added a lot of risk into a patch series that is alrea= dy touching a lot of sensitive areas, and therefore introducing a lot o= f risks. Once this patch series is hopefully intergrated, flush subnorm= als will be much easier to integrate, since it will be mips-only issue.= Therefore, if you agree, I will leave it for the future. I will defini= tely mention it in commit messages though (as a limitaion), for future = reference. Thanks again for your consideration of this matter. Sincerely yours, Aleksandar -------- Original Message -------- Subject: Re: [PATCH 2/2] target-mips: Implement IEEE 754-2008 functiona= lity for R6 and MSA instructions Date: Friday, April 1, 2016 21:07 CEST From: Leon Alrae To: Aleksandar Markovic , CC: , , ,, , ,, , ,<= proljc@gmail.com>, , ,, ,= , ,, References: <1458910214-12239-1-git-send-email-aleksandar.markovic@rt-r= k.com><1458910214-12239-3-git-send-email-aleksandar.markovic@rt-rk.com>= =C2=A0On 25/03/16 12:50, Aleksandar Markovic wrote: > +#define MSA=5FCLASS=5FSIGNALING=5FNAN 0x001 > +#define MSA=5FCLASS=5FQUIET=5FNAN 0x002 > +#define MSA=5FCLASS=5FNEGATIVE=5FINFINITY 0x004 > +#define MSA=5FCLASS=5FNEGATIVE=5FNORMAL 0x008 > +#define MSA=5FCLASS=5FNEGATIVE=5FSUBNORMAL 0x010 > +#define MSA=5FCLASS=5FNEGATIVE=5FZERO 0x020 > +#define MSA=5FCLASS=5FPOSITIVE=5FINFINITY 0x040 > +#define MSA=5FCLASS=5FPOSITIVE=5FNORMAL 0x080 > +#define MSA=5FCLASS=5FPOSITIVE=5FSUBNORMAL 0x100 > +#define MSA=5FCLASS=5FPOSITIVE=5FZERO 0x200 > + > +#define MSA=5FCLASS(name, bits) \ > +uint ## bits ## =5Ft helper=5Fmsa=5F ## name (CPUMIPSState *env, \ > + uint ## bits ## =5Ft arg) \ > +{ \ > + if (float ## bits ## =5Fis=5Fsignaling=5Fnan(arg, \ > + &env->active=5Ftc.msa=5Ffp=5Fstatus)) { \ > + return MSA=5FCLASS=5FSIGNALING=5FNAN; \ > + } else if (float ## bits ## =5Fis=5Fquiet=5Fnan(arg, \ > + &env->active=5Ftc.msa=5Ffp=5Fstatus)) { \ > + return MSA=5FCLASS=5FQUIET=5FNAN; \ > + } else if (float ## bits ## =5Fis=5Fneg(arg)) { \ > + if (float ## bits ## =5Fis=5Finfinity(arg)) { \ > + return MSA=5FCLASS=5FNEGATIVE=5FINFINITY; \ > + } else if (float ## bits ## =5Fis=5Fzero(arg)) { \ > + return MSA=5FCLASS=5FNEGATIVE=5FZERO; \ > + } else if (float ## bits ## =5Fis=5Fzero=5For=5Fdenormal(arg)) { \ > + return MSA=5FCLASS=5FNEGATIVE=5FSUBNORMAL; \ > + } else { \ > + return MSA=5FCLASS=5FNEGATIVE=5FNORMAL; \ > + } \ > + } else { \ > + if (float ## bits ## =5Fis=5Finfinity(arg)) { \ > + return MSA=5FCLASS=5FPOSITIVE=5FINFINITY; \ > + } else if (float ## bits ## =5Fis=5Fzero(arg)) { \ > + return MSA=5FCLASS=5FPOSITIVE=5FZERO; \ > + } else if (float ## bits ## =5Fis=5Fzero=5For=5Fdenormal(arg)) { \ > + return MSA=5FCLASS=5FPOSITIVE=5FSUBNORMAL; \ > + } else { \ > + return MSA=5FCLASS=5FPOSITIVE=5FNORMAL; \ > + } \ > + } \ > +} Duplicating the class operation is unnecessary. We can just have common= function for FPU and MSA which takes additional float=5Fstatus argument= . Also I noticed that this patch series doesn't provide Flush Subnormals (the FCSR.FS bit), but probably this functionality can come later... Leon =C2=A0 ------=_=-_OpenGroupware_org_NGMime-1932-1459695918.227686-0------ content-type: text/html; charset=utf-8 content-length: 6028 content-transfer-encoding: quoted-printable Hello, Leon, thank you very much for the kind feedback. Let me cl= arify my take on the involved issues.

1) Class operations
I am going to correct the code as you hinted.

The r= eason I wanted separate handling of MSA class operation is code and mod= ule decoupling. Handling of MSA instructions (in file msa=5Fhelper.c) a= nd regular instructions (in file op=5Fhelper.c) have many overlaping ar= eas - however, my understanding is that the designer of MSA module want= ed it to be as independant on code in other files/modulas as possible. = Handling class operation is on of the rare instances where code in msa=5F= helper.c relies on the code in op=5Fhelper.c., and it made sense to me = that this dependence should be removed, for the sake of consistency wit= hin MSA module - even if the functionalitied are virtually identical. T= hat said, I will anyway listen to your advice, since you most probably = see more than myself regarding this, and I am going to revert to a sing= le handling of class operations, for both MSA and regular versions.

2) Flush subnormals

My impression is that his set of= features should be treated and implemented separately, at some later p= oint in time.

Although the implementation seems not to be to= o complex (defining FCR31=5FFS, invoking appropriately set=5Fflush= =5Fto=5Fzero() and set=5Fflush=5Finputs=5Fto=5Fzero() on CPU init,= plus special exception handling, like it is already done for MSA equiv= alents), it looks to me that it would have added a lot of risk into a p= atch series that is already touching a lot of sensitive areas, and ther= efore introducing a lot of risks. Once this patch series is hopefully i= ntergrated, flush subnormals will be much easier to integrate, since it= will be mips-only issue. Therefore, if you agree, I will leave it for = the future. I will definitely mention it in commit messages though (as = a limitaion), for future reference.

Thanks again for your co= nsideration of this matter.

Sincerely yours,
Aleksandar=


-------- Original Message --------
Subject: Re: = [PATCH 2/2] target-mips: Implement IEEE 754-2008 functionality for R6 a= nd MSA instructions
Date: Friday, April 1, 2016 21:07 CEST
Fr= om: Leon Alrae <leon.alrae@imgtec.com>
To: Aleksandar Markov= ic <aleksandar.markovic@rt-rk.com>,<qemu-devel@nongnu.org><= br />CC: <qemu-arm@nongnu.org>, <qemu-ppc@nongnu.org>, <= aurelien@aurel32.net>,<peter.maydell@linaro.org>, <rth@twid= dle.net>, <afaerber@suse.de>,<pbonzini@redhat.com>, <= ehabkost@redhat.com>, <edgar.iglesias@gmail.com>,<proljc@gm= ail.com>, <agraf@suse.de>, <blauwirbel@gmail.com>,<ma= rk.cave-ayland@ilande.co.uk>, <gxt@mprc.pku.edu.cn>,<petar.= jovanovic@imgtec.com>, <miodrag.dinic@imgtec.com>,<jcmvbkbc= @gmail.com>, <kbastian@mail.uni-paderborn.de>
References:= <1458910214-12239-1-git-send-email-aleksandar.markovic@rt-rk.com>= ;<1458910214-12239-3-git-send-email-aleksandar.markovic@rt-rk.com>= ;


 
On 25/03/16 12:50, Aleksandar Markovic wrote:
>= ; +#define MSA=5FCLASS=5FSIGNALING=5FNAN 0x001
> +#define MSA=5F= CLASS=5FQUIET=5FNAN 0x002
> +#define MSA=5FCLASS=5FNEGATIVE=5FI= NFINITY 0x004
> +#define MSA=5FCLASS=5FNEGATIVE=5FNORMAL 0x008<= br />> +#define MSA=5FCLASS=5FNEGATIVE=5FSUBNORMAL 0x010
> += #define MSA=5FCLASS=5FNEGATIVE=5FZERO 0x020
> +#define MSA=5FCL= ASS=5FPOSITIVE=5FINFINITY 0x040
> +#define MSA=5FCLASS=5FPOSITI= VE=5FNORMAL 0x080
> +#define MSA=5FCLASS=5FPOSITIVE=5FSUBNORMAL= 0x100
> +#define MSA=5FCLASS=5FPOSITIVE=5FZERO 0x200
>= +
> +#define MSA=5FCLASS(name, bits) \
> +uint ## bits= ## =5Ft helper=5Fmsa=5F ## name (CPUMIPSState *env, \
> + uint= ## bits ## =5Ft arg) \
> +{ \
> + if (float ## bits ##= =5Fis=5Fsignaling=5Fnan(arg, \
> + &env->active=5Ftc.ms= a=5Ffp=5Fstatus)) { \
> + return MSA=5FCLASS=5FSIGNALING=5FNAN;= \
> + } else if (float ## bits ## =5Fis=5Fquiet=5Fnan(arg, \> + &env->active=5Ftc.msa=5Ffp=5Fstatus)) { \
> += return MSA=5FCLASS=5FQUIET=5FNAN; \
> + } else if (float ## bi= ts ## =5Fis=5Fneg(arg)) { \
> + if (float ## bits ## =5Fis=5Fin= finity(arg)) { \
> + return MSA=5FCLASS=5FNEGATIVE=5FINFINITY; = \
> + } else if (float ## bits ## =5Fis=5Fzero(arg)) { \
&= gt; + return MSA=5FCLASS=5FNEGATIVE=5FZERO; \
> + } else if (fl= oat ## bits ## =5Fis=5Fzero=5For=5Fdenormal(arg)) { \
> + retur= n MSA=5FCLASS=5FNEGATIVE=5FSUBNORMAL; \
> + } else { \
>= ; + return MSA=5FCLASS=5FNEGATIVE=5FNORMAL; \
> + } \
>= + } else { \
> + if (float ## bits ## =5Fis=5Finfinity(arg)) {= \
> + return MSA=5FCLASS=5FPOSITIVE=5FINFINITY; \
> + = } else if (float ## bits ## =5Fis=5Fzero(arg)) { \
> + return M= SA=5FCLASS=5FPOSITIVE=5FZERO; \
> + } else if (float ## bits ##= =5Fis=5Fzero=5For=5Fdenormal(arg)) { \
> + return MSA=5FCLASS=5F= POSITIVE=5FSUBNORMAL; \
> + } else { \
> + return MSA=5F= CLASS=5FPOSITIVE=5FNORMAL; \
> + } \
> + } \
> = +}

Duplicating the class operation is unnecessary. We can ju= st have common
function for FPU and MSA which takes additional flo= at=5Fstatus argument.

Also I noticed that this patch series = doesn't provide Flush Subnormals
(the FCSR.FS bit), but probab= ly this functionality can come later...

Leon


  ------=_=-_OpenGroupware_org_NGMime-1932-1459695918.227686-0--------