[PATCH 1/2] MIPS: math-emu: Do not export function `srl128`

[PATCH 1/2] MIPS: math-emu: Do not export function `srl128`

[Mathieu Malaterre]

Fix non-fatal warning:

arch/mips/math-emu/dp_maddf.c:19:6: warning: no previous prototype for ‘srl128’ [-Wmissing-prototypes]
 void srl128(u64 *hptr, u64 *lptr, int count)

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/mips/math-emu/dp_maddf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/math-emu/dp_maddf.c b/arch/mips/math-emu/dp_maddf.c
index 7ad79ed411f5..0e2278a47f43 100644
--- a/arch/mips/math-emu/dp_maddf.c
+++ b/arch/mips/math-emu/dp_maddf.c
@@ -16,7 +16,7 @@
 
 
 /* 128 bits shift right logical with rounding. */
-void srl128(u64 *hptr, u64 *lptr, int count)
+static void srl128(u64 *hptr, u64 *lptr, int count)
 {
 	u64 low;
 
-- 
2.11.0

[PATCH 2/2] MIPS: math-emu: Declare ys variable as possibly unused

[Mathieu Malaterre]

Fix non-fatal warning:

arch/mips/math-emu/sp_fdp.c: In function ‘ieee754sp_fdp’:
arch/mips/math-emu/ieee754int.h:60:31: warning: variable ‘ys’ set but not used [-Wunused-but-set-variable]
  unsigned int ym; int ye; int ys; int yc
                               ^
arch/mips/math-emu/sp_fdp.c:37:2: note: in expansion of macro ‘COMPYSP’
  COMPYSP;
  ^~~~~~~

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/mips/math-emu/ieee754int.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/math-emu/ieee754int.h b/arch/mips/math-emu/ieee754int.h
index 06ac0e2ac7ac..cb8f04cd24bf 100644
--- a/arch/mips/math-emu/ieee754int.h
+++ b/arch/mips/math-emu/ieee754int.h
@@ -57,7 +57,7 @@ static inline int ieee754_class_nan(int xc)
 	unsigned int xm; int xe; int xs __maybe_unused; int xc
 
 #define COMPYSP \
-	unsigned int ym; int ye; int ys; int yc
+	unsigned int ym; int ye; int ys __maybe_unused; int yc
 
 #define COMPZSP \
 	unsigned int zm; int ze; int zs; int zc
-- 
2.11.0

RE: [PATCH 2/2] MIPS: math-emu: Declare ys variable as possibly unused

[Aleksandar Markovic]

> Fix non-fatal warning:
>
> arch/mips/math-emu/sp_fdp.c: In function ‘ieee754sp_fdp’:
> arch/mips/math-emu/ieee754int.h:60:31: warning: variable ‘ys’ set but not used [-Wunused-but-set-variable]
>   unsigned int ym; int ye; int ys; int yc
>                                ^
> arch/mips/math-emu/sp_fdp.c:37:2: note: in expansion of macro ‘COMPYSP’
>   COMPYSP;
>   ^~~~~~~
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>  arch/mips/math-emu/ieee754int.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/math-emu/ieee754int.h b/arch/mips/math-emu/ieee754int.h
> index 06ac0e2ac7ac..cb8f04cd24bf 100644
> --- a/arch/mips/math-emu/ieee754int.h
> +++ b/arch/mips/math-emu/ieee754int.h
> @@ -57,7 +57,7 @@ static inline int ieee754_class_nan(int xc)
>          unsigned int xm; int xe; int xs __maybe_unused; int xc
>  
>  #define COMPYSP \
> -       unsigned int ym; int ye; int ys; int yc
> +       unsigned int ym; int ye; int ys __maybe_unused; int yc
>  
>  #define COMPZSP \
>          unsigned int zm; int ze; int zs; int zc

This will silence the warning, but will do it for all future cases of unused
ys too - in other words, it may well silence even useful, valid warnings.
Also, this introduces an inconsistency among COMPXSP, COMPYSP, and COMPZSP
macros.

A better solution would be to reduce the scope of ys, so that it is always
used, if declared. Instead of this code segment (in arch/mips/math-emu/sp_fdp.c):

union ieee754sp ieee754sp_fdp(union ieee754dp x)
{
	union ieee754sp y;
	u32 rm;

	COMPXDP;
	COMPYSP;

	EXPLODEXDP;

	ieee754_clearcx();

	FLUSHXDP;

	switch (xc) {
	case IEEE754_CLASS_SNAN:
		x = ieee754dp_nanxcpt(x);
		EXPLODEXDP;
		/* Fall through.  */
	case IEEE754_CLASS_QNAN:
		y = ieee754sp_nan_fdp(xs, xm);
		if (!ieee754_csr.nan2008) {
			EXPLODEYSP;
			if (!ieee754_class_nan(yc))
				y = ieee754sp_indef();
		}
		return y;


.. should be the following: (COMPYSP is moved to a smaller code block)

union ieee754sp ieee754sp_fdp(union ieee754dp x)
{
	union ieee754sp y;
	u32 rm;

	COMPXDP;	

	EXPLODEXDP;

	ieee754_clearcx();

	FLUSHXDP;

	switch (xc) {
	case IEEE754_CLASS_SNAN:
		x = ieee754dp_nanxcpt(x);
		EXPLODEXDP;
		/* Fall through.  */
	case IEEE754_CLASS_QNAN:
		{
			COMPYSP;

			y = ieee754sp_nan_fdp(xs, xm);
			if (!ieee754_csr.nan2008) {
				EXPLODEYSP;
				if (!ieee754_class_nan(yc))
					y = ieee754sp_indef();
			}
			return y;
		}

Regards,
Aleksandar

RE: [PATCH 1/2] MIPS: math-emu: Do not export function `srl128`

[Aleksandar Markovic]

> Fix non-fatal warning:
> 
> arch/mips/math-emu/dp_maddf.c:19:6: warning: no previous prototype for ‘srl128’ [-Wmissing-prototypes]
>  void srl128(u64 *hptr, u64 *lptr, int count)
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>  arch/mips/math-emu/dp_maddf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/math-emu/dp_maddf.c b/arch/mips/math-emu/dp_maddf.c
> index 7ad79ed411f5..0e2278a47f43 100644
> --- a/arch/mips/math-emu/dp_maddf.c
> +++ b/arch/mips/math-emu/dp_maddf.c
> @@ -16,7 +16,7 @@
>  
>  
>  /* 128 bits shift right logical with rounding. */
> -void srl128(u64 *hptr, u64 *lptr, int count)
> +static void srl128(u64 *hptr, u64 *lptr, int count)
>  {
>          u64 low;
>  
> -- 
> 2.11.0

Acked-by: Aleksandar Markovic <aleksandar.markovic@mips.com>

RE: [PATCH 1/2] MIPS: math-emu: Do not export function `srl128`

[Aleksandar Markovic]

> > Fix non-fatal warning:
> >
> > arch/mips/math-emu/dp_maddf.c:19:6: warning: no previous prototype for ‘srl128’ [-Wmissing-prototypes]
> >  void srl128(u64 *hptr, u64 *lptr, int count)
> >
> > Signed-off-by: Mathieu Malaterre <malat@debian.org>
> > ---
> >  arch/mips/math-emu/dp_maddf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/mips/math-emu/dp_maddf.c b/arch/mips/math-emu/dp_maddf.c
> > index 7ad79ed411f5..0e2278a47f43 100644
> > --- a/arch/mips/math-emu/dp_maddf.c
> > +++ b/arch/mips/math-emu/dp_maddf.c
> > @@ -16,7 +16,7 @@
> >
> >
> >  /* 128 bits shift right logical with rounding. */
> > -void srl128(u64 *hptr, u64 *lptr, int count)
> > +static void srl128(u64 *hptr, u64 *lptr, int count)
> >  {
> >          u64 low;
> >
> > --
> > 2.11.0
> 
> Acked-by: Aleksandar Markovic <aleksandar.markovic@mips.com>

However, there is an already submitted patch: (the code change is identical)

https://www.linux-mips.org/archives/linux-mips/2017-11/msg00044.html

Status of that patch on patchwork is "Accepted".

Regards,
Aleksandar

Re: [PATCH 1/2] MIPS: math-emu: Do not export function `srl128`

[Mathieu Malaterre]

On Tue, Dec 26, 2017 at 7:10 PM, Aleksandar Markovic
<Aleksandar.Markovic@mips.com> wrote:
>> > Fix non-fatal warning:
>> >
>> > arch/mips/math-emu/dp_maddf.c:19:6: warning: no previous prototype for ‘srl128’ [-Wmissing-prototypes]
>> >  void srl128(u64 *hptr, u64 *lptr, int count)
>> >
>> > Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> > ---
>> >  arch/mips/math-emu/dp_maddf.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/arch/mips/math-emu/dp_maddf.c b/arch/mips/math-emu/dp_maddf.c
>> > index 7ad79ed411f5..0e2278a47f43 100644
>> > --- a/arch/mips/math-emu/dp_maddf.c
>> > +++ b/arch/mips/math-emu/dp_maddf.c
>> > @@ -16,7 +16,7 @@
>> >
>> >
>> >  /* 128 bits shift right logical with rounding. */
>> > -void srl128(u64 *hptr, u64 *lptr, int count)
>> > +static void srl128(u64 *hptr, u64 *lptr, int count)
>> >  {
>> >          u64 low;
>> >
>> > --
>> > 2.11.0
>>
>> Acked-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
>
> However, there is an already submitted patch: (the code change is identical)
>
> https://www.linux-mips.org/archives/linux-mips/2017-11/msg00044.html
>
> Status of that patch on patchwork is "Accepted".

Sorry did not realized you sent it already.

Thanks

Re: [PATCH 2/2] MIPS: math-emu: Declare ys variable as possibly unused

[Mathieu Malaterre]

Aleksandar,

On Tue, Dec 26, 2017 at 4:12 PM, Aleksandar Markovic
<Aleksandar.Markovic@mips.com> wrote:
>> Fix non-fatal warning:
>>
>> arch/mips/math-emu/sp_fdp.c: In function ‘ieee754sp_fdp’:
>> arch/mips/math-emu/ieee754int.h:60:31: warning: variable ‘ys’ set but not used [-Wunused-but-set-variable]
>>   unsigned int ym; int ye; int ys; int yc
>>                                ^
>> arch/mips/math-emu/sp_fdp.c:37:2: note: in expansion of macro ‘COMPYSP’
>>   COMPYSP;
>>   ^~~~~~~
>>
>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> ---
>>  arch/mips/math-emu/ieee754int.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/math-emu/ieee754int.h b/arch/mips/math-emu/ieee754int.h
>> index 06ac0e2ac7ac..cb8f04cd24bf 100644
>> --- a/arch/mips/math-emu/ieee754int.h
>> +++ b/arch/mips/math-emu/ieee754int.h
>> @@ -57,7 +57,7 @@ static inline int ieee754_class_nan(int xc)
>>          unsigned int xm; int xe; int xs __maybe_unused; int xc
>>
>>  #define COMPYSP \
>> -       unsigned int ym; int ye; int ys; int yc
>> +       unsigned int ym; int ye; int ys __maybe_unused; int yc
>>
>>  #define COMPZSP \
>>          unsigned int zm; int ze; int zs; int zc
>
> This will silence the warning, but will do it for all future cases of unused
> ys too - in other words, it may well silence even useful, valid warnings.
> Also, this introduces an inconsistency among COMPXSP, COMPYSP, and COMPZSP
> macros.
>
> A better solution would be to reduce the scope of ys, so that it is always
> used, if declared. Instead of this code segment (in arch/mips/math-emu/sp_fdp.c):
>
> union ieee754sp ieee754sp_fdp(union ieee754dp x)
> {
>         union ieee754sp y;
>         u32 rm;
>
>         COMPXDP;
>         COMPYSP;
>
>         EXPLODEXDP;
>
>         ieee754_clearcx();
>
>         FLUSHXDP;
>
>         switch (xc) {
>         case IEEE754_CLASS_SNAN:
>                 x = ieee754dp_nanxcpt(x);
>                 EXPLODEXDP;
>                 /* Fall through.  */
>         case IEEE754_CLASS_QNAN:
>                 y = ieee754sp_nan_fdp(xs, xm);
>                 if (!ieee754_csr.nan2008) {
>                         EXPLODEYSP;
>                         if (!ieee754_class_nan(yc))
>                                 y = ieee754sp_indef();
>                 }
>                 return y;
>
>
> ... should be the following: (COMPYSP is moved to a smaller code block)
>
> union ieee754sp ieee754sp_fdp(union ieee754dp x)
> {
>         union ieee754sp y;
>         u32 rm;
>
>         COMPXDP;
>
>         EXPLODEXDP;
>
>         ieee754_clearcx();
>
>         FLUSHXDP;
>
>         switch (xc) {
>         case IEEE754_CLASS_SNAN:
>                 x = ieee754dp_nanxcpt(x);
>                 EXPLODEXDP;
>                 /* Fall through.  */
>         case IEEE754_CLASS_QNAN:
>                 {
>                         COMPYSP;
>
>                         y = ieee754sp_nan_fdp(xs, xm);
>                         if (!ieee754_csr.nan2008) {
>                                 EXPLODEYSP;
>                                 if (!ieee754_class_nan(yc))
>                                         y = ieee754sp_indef();
>                         }
>                         return y;
>                 }
>


Thanks for the suggestion. However the sign bit is still not used, so
the warning is still there. Just for clarity did you see that:

 #define COMPXSP \
       unsigned int xm; int xe; int xs __maybe_unused; int xc

I'll try to give it some more thoughts, and come up with something
hopefully working.

-M