[PATCH]: fix compiler warnings in the math-emulator

[PATCH]: fix compiler warnings in the math-emulator

[Juan Quintela]

Hi
        this patch does:

* redefine SETCX to only set cx
* define a new macre SETANDTESTCX for the few cases when we also want to
  test the value set.

Later, Juan.

Index: arch/mips/math-emu/dp_cmp.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/math-emu/dp_cmp.c,v
retrieving revision 1.4.2.1
diff -u -r1.4.2.1 dp_cmp.c
--- arch/mips/math-emu/dp_cmp.c	5 Aug 2002 14:18:22 -0000	1.4.2.1
+++ arch/mips/math-emu/dp_cmp.c	18 Dec 2002 00:49:18 -0000
@@ -44,7 +44,7 @@
 		if (cmp & IEEE754_CUN)
 			return 1;
 		if (cmp & (IEEE754_CLT | IEEE754_CGT)) {
-			if (sig && SETCX(IEEE754_INVALID_OPERATION))
+			if (sig && SETANDTESTCX(IEEE754_INVALID_OPERATION))
 				return ieee754si_xcpt(0, "fcmpf", x);
 		}
 		return 0;
Index: arch/mips/math-emu/ieee754dp.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/math-emu/ieee754dp.c,v
retrieving revision 1.5.2.3
diff -u -r1.5.2.3 ieee754dp.c
--- arch/mips/math-emu/ieee754dp.c	5 Aug 2002 23:53:34 -0000	1.5.2.3
+++ arch/mips/math-emu/ieee754dp.c	18 Dec 2002 00:49:18 -0000
@@ -69,7 +69,7 @@
 	if (!ieee754dp_issnan(r))	/* QNAN does not cause invalid op !! */
 		return r;
 
-	if (!SETCX(IEEE754_INVALID_OPERATION)) {
+	if (!SETANDTESTCX(IEEE754_INVALID_OPERATION)) {
 		/* not enabled convert to a quiet NaN */
 		DPMANT(r) &= (~DP_MBIT(DP_MBITS-1));
 		if (ieee754dp_isnan(r))
Index: arch/mips/math-emu/ieee754int.h
===================================================================
RCS file: /home/cvs/linux/arch/mips/math-emu/ieee754int.h,v
retrieving revision 1.3.2.2
diff -u -r1.3.2.2 ieee754int.h
--- arch/mips/math-emu/ieee754int.h	5 Aug 2002 23:53:34 -0000	1.3.2.2
+++ arch/mips/math-emu/ieee754int.h	18 Dec 2002 00:49:18 -0000
@@ -61,7 +61,10 @@
   (ieee754_csr.cx = 0)
 
 #define SETCX(x) \
-  (ieee754_csr.cx |= (x),ieee754_csr.sx |= (x),ieee754_csr.mx & (x))
+  (ieee754_csr.cx |= (x),ieee754_csr.sx |= (x))
+
+#define SETANDTESTCX(x) \
+  (SETCX(x),ieee754_csr.mx & (x))
 
 #define TSTX()	\
 	(ieee754_csr.cx & ieee754_csr.mx)
Index: arch/mips/math-emu/ieee754sp.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/math-emu/ieee754sp.c,v
retrieving revision 1.6.2.2
diff -u -r1.6.2.2 ieee754sp.c
--- arch/mips/math-emu/ieee754sp.c	5 Aug 2002 23:53:34 -0000	1.6.2.2
+++ arch/mips/math-emu/ieee754sp.c	18 Dec 2002 00:49:18 -0000
@@ -70,7 +70,7 @@
 	if (!ieee754sp_issnan(r))	/* QNAN does not cause invalid op !! */
 		return r;
 
-	if (!SETCX(IEEE754_INVALID_OPERATION)) {
+	if (!SETANDTESTCX(IEEE754_INVALID_OPERATION)) {
 		/* not enabled convert to a quiet NaN */
 		SPMANT(r) &= (~SP_MBIT(SP_MBITS-1));
 		if (ieee754sp_isnan(r))
Index: arch/mips/math-emu/sp_cmp.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/math-emu/sp_cmp.c,v
retrieving revision 1.4.2.1
diff -u -r1.4.2.1 sp_cmp.c
--- arch/mips/math-emu/sp_cmp.c	5 Aug 2002 14:18:23 -0000	1.4.2.1
+++ arch/mips/math-emu/sp_cmp.c	18 Dec 2002 00:49:18 -0000
@@ -44,7 +44,7 @@
 		if (cmp & IEEE754_CUN)
 			return 1;
 		if (cmp & (IEEE754_CLT | IEEE754_CGT)) {
-			if (sig && SETCX(IEEE754_INVALID_OPERATION))
+			if (sig && SETANDTESTCX(IEEE754_INVALID_OPERATION))
 				return ieee754si_xcpt(0, "fcmpf", x);
 		}
 		return 0;
-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [PATCH]: fix compiler warnings in the math-emulator

[Maciej W. Rozycki]

On 18 Dec 2002, Juan Quintela wrote:

>         this patch does:
> 
> * redefine SETCX to only set cx
> * define a new macre SETANDTESTCX for the few cases when we also want to
>   test the value set.

 Is it needed?  The part that returns .mx should be optimized away by the
compiler automagically if unused. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [PATCH]: fix compiler warnings in the math-emulator

[Juan Quintela]

>>>>> "maciej" == Maciej W Rozycki <macro@ds2.pg.gda.pl> writes:

maciej> On 18 Dec 2002, Juan Quintela wrote:
>> this patch does:
>> 
>> * redefine SETCX to only set cx
>> * define a new macre SETANDTESTCX for the few cases when we also want to
>> test the value set.

maciej> Is it needed?  The part that returns .mx should be optimized away by the
maciej> compiler automagically if unused. 

Idea was to make things compile without warnings, that way when you
change anything, you search for warnings :(

With the changes that I sent, I have put the warnings levels down to
(for IP22) to:
     - 7 C warnings
     - 2 Asm warnings

And some of the remaining warnings are printk(), where we mix ints &
longs all around (no problem if the code is not the same than in 64
bits, otherwise, problem :(

Later, Juan.

-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [PATCH]: fix compiler warnings in the math-emulator

[Maciej W. Rozycki]

On 18 Dec 2002, Juan Quintela wrote:

> maciej> Is it needed?  The part that returns .mx should be optimized away by the
> maciej> compiler automagically if unused. 
> 
> Idea was to make things compile without warnings, that way when you
> change anything, you search for warnings :(

 The idea is fine, sure.

> With the changes that I sent, I have put the warnings levels down to
> (for IP22) to:
>      - 7 C warnings
>      - 2 Asm warnings

 A few warnings are unavoidable -- e.g. there is no way to shut up gas
complaining about macros expanding into multiple instructions in branch
delay slots.  Too bad.

 How about this patch? -- it seems to work here (gcc 2.95.4).

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

patch-mips-2.4.20-pre6-20021212-setcx-0
diff -up --recursive --new-file linux-mips-2.4.20-pre6-20021212.macro/arch/mips/math-emu/ieee754int.h linux-mips-2.4.20-pre6-20021212/arch/mips/math-emu/ieee754int.h
--- linux-mips-2.4.20-pre6-20021212.macro/arch/mips/math-emu/ieee754int.h	2002-12-16 17:17:55.000000000 +0000
+++ linux-mips-2.4.20-pre6-20021212/arch/mips/math-emu/ieee754int.h	2002-12-18 18:31:51.000000000 +0000
@@ -58,10 +58,10 @@
 #define CLPAIR(x,y)	((x)*6+(y))
 
 #define CLEARCX	\
-  (ieee754_csr.cx = 0)
+	(ieee754_csr.cx = 0)
 
 #define SETCX(x) \
-  (ieee754_csr.cx |= (x),ieee754_csr.sx |= (x),ieee754_csr.mx & (x))
+	({ieee754_csr.cx |= (x); ieee754_csr.sx |= (x); ieee754_csr.mx & (x);})
 
 #define TSTX()	\
 	(ieee754_csr.cx & ieee754_csr.mx)

Re: [PATCH]: fix compiler warnings in the math-emulator

[Greg Lindahl]

On Wed, Dec 18, 2002 at 07:47:31PM +0100, Maciej W. Rozycki wrote:

>  A few warnings are unavoidable -- e.g. there is no way to shut up gas
> complaining about macros expanding into multiple instructions in branch
> delay slots.  Too bad.

... why isn't that a bug?

greg

Re: [PATCH]: fix compiler warnings in the math-emulator

[Hartvig Ekner]

Sometimes you don't care whether you do only "half" a macro instruction
if the branch is taken. Usually though, the warning is a good thing - I
remember having spent many hours finding bugs like this with assemblers
that don't issue warnings.

/Hartvig

Greg Lindahl writes:
> 
> On Wed, Dec 18, 2002 at 07:47:31PM +0100, Maciej W. Rozycki wrote:
> 
> >  A few warnings are unavoidable -- e.g. there is no way to shut up gas
> > complaining about macros expanding into multiple instructions in branch
> > delay slots.  Too bad.
> 
> ... why isn't that a bug?
> 
> greg

Re: [PATCH]: fix compiler warnings in the math-emulator

[Hartvig Ekner]

Sometimes you don't care whether you do only "half" a macro instruction
if the branch is taken. Usually though, the warning is a good thing - I
remember having spent many hours finding bugs like this with assemblers
that don't issue warnings.

/Hartvig

Greg Lindahl writes:
> 
> On Wed, Dec 18, 2002 at 07:47:31PM +0100, Maciej W. Rozycki wrote:
> 
> >  A few warnings are unavoidable -- e.g. there is no way to shut up gas
> > complaining about macros expanding into multiple instructions in branch
> > delay slots.  Too bad.
> 
> ... why isn't that a bug?
> 
> greg

Re: [PATCH]: fix compiler warnings in the math-emulator

[Maciej W. Rozycki]

On Wed, 18 Dec 2002, Hartvig Ekner wrote:

> Sometimes you don't care whether you do only "half" a macro instruction
> if the branch is taken. Usually though, the warning is a good thing - I
> remember having spent many hours finding bugs like this with assemblers
> that don't issue warnings.

 This is exactly what ".set nomacro" is for -- I can't see any reason for
such warnings when ".set macro" is active.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [PATCH]: fix compiler warnings in the math-emulator

[Greg Lindahl]

> > Sometimes you don't care whether you do only "half" a macro instruction
> > if the branch is taken. Usually though, the warning is a good thing - I
> > remember having spent many hours finding bugs like this with assemblers
> > that don't issue warnings.
> 
>  This is exactly what ".set nomacro" is for -- I can't see any reason for
> such warnings when ".set macro" is active.

While in general I like playing with sharp knives (you should see my
cluster admin toolkit), and I like my kernel to run fast, and the only
thing that gets trashed is usually at, I still think I'd prefer to
have nops in those delay slots in the kernel. Hand-written assembly is
unlikely to use ".set nomacro" properly.

-- greg

Re: [PATCH]: fix compiler warnings in the math-emulator

[Juan Quintela]

>>>>> "greg" == Greg Lindahl <lindahl@keyresearch.com> writes:

>> > Sometimes you don't care whether you do only "half" a macro instruction
>> > if the branch is taken. Usually though, the warning is a good thing - I
>> > remember having spent many hours finding bugs like this with assemblers
>> > that don't issue warnings.
>> 
>> This is exactly what ".set nomacro" is for -- I can't see any reason for
>> such warnings when ".set macro" is active.

greg> While in general I like playing with sharp knives (you should see my
greg> cluster admin toolkit), and I like my kernel to run fast, and the only
greg> thing that gets trashed is usually at, I still think I'd prefer to
greg> have nops in those delay slots in the kernel. Hand-written assembly is
greg> unlikely to use ".set nomacro" properly.

In the kernel for IP22 (I suppose the same for other architectures),
it has only two Assembler warnings.  And they are all in the fast
path :(

one is in entry.S:excetp_vec3_r4000


and the other is in head.S:ejtag_debug_handler

I am not sure about the first one, but the second one shouldn't be
performance critical :(

Later, Juan.

-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [PATCH]: fix compiler warnings in the math-emulator

[Maciej W. Rozycki]

On 18 Dec 2002, Juan Quintela wrote:

> In the kernel for IP22 (I suppose the same for other architectures),
> it has only two Assembler warnings.  And they are all in the fast
> path :(
> 
> one is in entry.S:excetp_vec3_r4000

 That's damn performace-critical.  Thats why the VCED and VCEI handlers
are inlined there (unlike the rest) in the first place.

> and the other is in head.S:ejtag_debug_handler

 That's non-critical, but I see no reason to degrade code because of
deficiencies of tools.  Tools should be fixed instead. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [PATCH]: fix compiler warnings in the math-emulator

[Juan Quintela]

>>>>> "maciej" == Maciej W Rozycki <macro@ds2.pg.gda.pl> writes:

maciej> On 18 Dec 2002, Juan Quintela wrote:
maciej> Is it needed?  The part that returns .mx should be optimized away by the
maciej> compiler automagically if unused. 
>> 
>> Idea was to make things compile without warnings, that way when you
>> change anything, you search for warnings :(

maciej> The idea is fine, sure.

>> With the changes that I sent, I have put the warnings levels down to
>> (for IP22) to:
>> - 7 C warnings
>> - 2 Asm warnings

maciej> A few warnings are unavoidable -- e.g. there is no way to shut up gas
maciej> complaining about macros expanding into multiple instructions in branch
maciej> delay slots.  Too bad.

I tried the 
  .set nowarn 
and found it didn't work.


maciej> How about this patch? -- it seems to work here (gcc 2.95.4).

Nope, the probem is that in the 3 places that I use the SETANDTEST()
macro they need the returned value.

Later, Juan.

maciej> -- 
maciej> +  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
maciej> +--------------------------------------------------------------+
maciej> +        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

maciej> patch-mips-2.4.20-pre6-20021212-setcx-0
maciej> diff -up --recursive --new-file linux-mips-2.4.20-pre6-20021212.macro/arch/mips/math-emu/ieee754int.h linux-mips-2.4.20-pre6-20021212/arch/mips/math-emu/ieee754int.h
maciej> --- linux-mips-2.4.20-pre6-20021212.macro/arch/mips/math-emu/ieee754int.h	2002-12-16 17:17:55.000000000 +0000
maciej> +++ linux-mips-2.4.20-pre6-20021212/arch/mips/math-emu/ieee754int.h	2002-12-18 18:31:51.000000000 +0000
maciej> @@ -58,10 +58,10 @@
maciej> #define CLPAIR(x,y)	((x)*6+(y))
 
maciej> #define CLEARCX	\
maciej> -  (ieee754_csr.cx = 0)
maciej> +	(ieee754_csr.cx = 0)
 
maciej> #define SETCX(x) \
maciej> -  (ieee754_csr.cx |= (x),ieee754_csr.sx |= (x),ieee754_csr.mx & (x))
maciej> +	({ieee754_csr.cx |= (x); ieee754_csr.sx |= (x); ieee754_csr.mx & (x);})
 
maciej> #define TSTX()	\
maciej> (ieee754_csr.cx & ieee754_csr.mx)


-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy

Re: [PATCH]: fix compiler warnings in the math-emulator

[Maciej W. Rozycki]

On 18 Dec 2002, Juan Quintela wrote:

> maciej> A few warnings are unavoidable -- e.g. there is no way to shut up gas
> maciej> complaining about macros expanding into multiple instructions in branch
> maciej> delay slots.  Too bad.
> 
> I tried the 
>   .set nowarn 
> and found it didn't work.

 There is no such gas directive, but perhaps it might be added as writing
".set nomacro" before every branch may be a nuisance and obfuscate the
code unnecessarily.  On the contrary ".set nowarn" should be fairly rare. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [PATCH]: fix compiler warnings in the math-emulator

[Ralf Baechle]

On Wed, Dec 18, 2002 at 05:43:34PM +0100, Maciej W. Rozycki wrote:

>  Is it needed?  The part that returns .mx should be optimized away by the
> compiler automagically if unused. 

Functionally these two patches are more or less equivalent but I think
Juan's patch is more readable.  Don't change the fact that the whole FP
emulator could need some heavy polishing ...

  Ralf

Re: [PATCH]: fix compiler warnings in the math-emulator

[Maciej W. Rozycki]

On Thu, 19 Dec 2002, Ralf Baechle wrote:

> Functionally these two patches are more or less equivalent but I think
> Juan's patch is more readable.  Don't change the fact that the whole FP
> emulator could need some heavy polishing ...

 If the additional macro is OK, then I see no problem.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +