[PATCH] math-emu/setcc: avoid gcc extension

[PATCH] math-emu/setcc: avoid gcc extension

[Randy Dunlap]

From: Randy Dunlap <randy.dunlap@oracle.com>

setcc() in math-emu is written as a gcc extension statement expression
macro that returns a value.  However, it's not used that way and it's
not needed like that, so just make it a do-while non-extension macro
so that we don't use an extension when it's not needed.

All 4 .c files that use setcc() produce the same code after this patch.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 arch/i386/math-emu/status_w.h        |    5 +++--

---
 arch/i386/math-emu/status_w.h |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- linux-2620-rc2.orig/arch/i386/math-emu/status_w.h
+++ linux-2620-rc2/arch/i386/math-emu/status_w.h
@@ -48,9 +48,10 @@
 
 #define status_word() \
   ((partial_status & ~SW_Top & 0xffff) | ((top << SW_Top_Shift) & SW_Top))
-#define setcc(cc) ({ \
+#define setcc(cc) do { \
   partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
-  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); })
+  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); } \
+	while (0)
 
 #ifdef PECULIAR_486
    /* Default, this conveys no information, but an 80486 does it. */


---

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Segher Boessenkool]

> setcc() in math-emu is written as a gcc extension statement expression
> macro that returns a value.  However, it's not used that way and it's
> not needed like that, so just make it a do-while non-extension macro
> so that we don't use an extension when it's not needed.

Looks fine, except

> -#define setcc(cc) ({ \
> +#define setcc(cc) do { \
>    partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
> -  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); })
> +  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); } \
> +	while (0)

closing brace on the "while" line, please.


Segher

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Randy Dunlap]

On Sun, 7 Jan 2007 14:27:32 +0100 Segher Boessenkool wrote:

> closing brace on the "while" line, please.

---
From: Randy Dunlap <randy.dunlap@oracle.com>

setcc() in math-emu is written as a gcc extension statement expression
macro that returns a value.  However, it's not used that way and it's
not needed like that, so just make it a do-while non-extension macro
so that we don't use an extension when it's not needed.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 arch/i386/math-emu/status_w.h        |    5 +++--

---
 arch/i386/math-emu/status_w.h |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- linux-2620-rc2.orig/arch/i386/math-emu/status_w.h
+++ linux-2620-rc2/arch/i386/math-emu/status_w.h
@@ -48,9 +48,10 @@
 
 #define status_word() \
   ((partial_status & ~SW_Top & 0xffff) | ((top << SW_Top_Shift) & SW_Top))
-#define setcc(cc) ({ \
+#define setcc(cc) do { \
   partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
-  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); })
+  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); \
+	} while (0)
 
 #ifdef PECULIAR_486
    /* Default, this conveys no information, but an 80486 does it. */

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Segher Boessenkool]

> -#define setcc(cc) ({ \
> +#define setcc(cc) do { \
>    partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
> -  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); })
> +  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); \
> +	} while (0)

There's an extra tab in that last line.  Could you also
please fix the indenting (use a tab, not spaces) -- I know
it was there originally, but since there are only a few
lines in that file like that...  :-)

[You must be tired of me by now, heh]

Thanks,


Segher

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Randy Dunlap]

On Sun, 7 Jan 2007 20:12:42 +0100 Segher Boessenkool wrote:

> There's an extra tab in that last line.  Could you also
> please fix the indenting (use a tab, not spaces) -- I know
> it was there originally, but since there are only a few
> lines in that file like that...  :-)

how's this one?
---
From: Randy Dunlap <randy.dunlap@oracle.com>

setcc() in math-emu is written as a gcc extension statement expression
macro that returns a value.  However, it's not used that way and it's
not needed like that, so just make it a do-while non-extension macro
so that we don't use an extension when it's not needed.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 arch/i386/math-emu/status_w.h        |    5 +++--

---
 arch/i386/math-emu/status_w.h |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

--- linux-2620-rc2.orig/arch/i386/math-emu/status_w.h
+++ linux-2620-rc2/arch/i386/math-emu/status_w.h
@@ -48,9 +48,10 @@
 
 #define status_word() \
   ((partial_status & ~SW_Top & 0xffff) | ((top << SW_Top_Shift) & SW_Top))
-#define setcc(cc) ({ \
-  partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
-  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); })
+#define setcc(cc) do { \
+	partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
+	partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); \
+} while (0)
 
 #ifdef PECULIAR_486
    /* Default, this conveys no information, but an 80486 does it. */

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Robert P. J. Day]

On Sun, 7 Jan 2007, Randy Dunlap wrote:

> On Sun, 7 Jan 2007 20:12:42 +0100 Segher Boessenkool wrote:
>
> > There's an extra tab in that last line.  Could you also
> > please fix the indenting (use a tab, not spaces) -- I know
> > it was there originally, but since there are only a few
> > lines in that file like that...  :-)
>
> how's this one?
> ---
> From: Randy Dunlap <randy.dunlap@oracle.com>
>
> setcc() in math-emu is written as a gcc extension statement
> expression macro that returns a value.  However, it's not used that
> way and it's not needed like that, so just make it a do-while
> non-extension macro so that we don't use an extension when it's not
> needed.

is that the proposed coding style for macros?  if it returns a value,
use "({ })"?  if not, use the "do ... while" notation?

rday

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Segher Boessenkool]

>> There's an extra tab in that last line.  Could you also
>> please fix the indenting (use a tab, not spaces) -- I know
>> it was there originally, but since there are only a few
>> lines in that file like that...  :-)
>
> how's this one?

I meant fix all the wrongly indented lines in that file (there
are only a few, and all around where you're patching anyway).

Care for one extra time?  :-)


Segher

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Randy Dunlap]

On Sun, 7 Jan 2007 20:29:21 +0100 Segher Boessenkool wrote:

> >> There's an extra tab in that last line.  Could you also
> >> please fix the indenting (use a tab, not spaces) -- I know
> >> it was there originally, but since there are only a few
> >> lines in that file like that...  :-)
> >
> > how's this one?
> 
> I meant fix all the wrongly indented lines in that file (there
> are only a few, and all around where you're patching anyway).
> 
> Care for one extra time?  :-)

Not really.  That should be a different patch IMO.

---
~Randy

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Christoph Hellwig]

On Sun, Jan 07, 2007 at 11:19:00AM -0800, Randy Dunlap wrote:
> On Sun, 7 Jan 2007 20:12:42 +0100 Segher Boessenkool wrote:
> 
> > There's an extra tab in that last line.  Could you also
> > please fix the indenting (use a tab, not spaces) -- I know
> > it was there originally, but since there are only a few
> > lines in that file like that...  :-)
> 
> how's this one?
> ---
> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> setcc() in math-emu is written as a gcc extension statement expression
> macro that returns a value.  However, it's not used that way and it's
> not needed like that, so just make it a do-while non-extension macro
> so that we don't use an extension when it's not needed.
> 
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> ---
>  arch/i386/math-emu/status_w.h        |    5 +++--
> 
> ---
>  arch/i386/math-emu/status_w.h |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> --- linux-2620-rc2.orig/arch/i386/math-emu/status_w.h
> +++ linux-2620-rc2/arch/i386/math-emu/status_w.h
> @@ -48,9 +48,10 @@
>  
>  #define status_word() \
>    ((partial_status & ~SW_Top & 0xffff) | ((top << SW_Top_Shift) & SW_Top))
> -#define setcc(cc) ({ \
> -  partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
> -  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); })
> +#define setcc(cc) do { \
> +	partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
> +	partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); \
> +} while (0)

Is there any reason you this shouldn't be an inline function?

Re: [PATCH] math-emu/setcc: avoid gcc extension

[Randy Dunlap]

On Sun, 7 Jan 2007 19:58:45 +0000 Christoph Hellwig wrote:

> On Sun, Jan 07, 2007 at 11:19:00AM -0800, Randy Dunlap wrote:
> > On Sun, 7 Jan 2007 20:12:42 +0100 Segher Boessenkool wrote:
> > 
> > > There's an extra tab in that last line.  Could you also
> > > please fix the indenting (use a tab, not spaces) -- I know
> > > it was there originally, but since there are only a few
> > > lines in that file like that...  :-)
> > 
> > how's this one?
> > ---
> > From: Randy Dunlap <randy.dunlap@oracle.com>
> > 
> > setcc() in math-emu is written as a gcc extension statement expression
> > macro that returns a value.  However, it's not used that way and it's
> > not needed like that, so just make it a do-while non-extension macro
> > so that we don't use an extension when it's not needed.
> > 
> > Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> > ---
> >  arch/i386/math-emu/status_w.h        |    5 +++--
> > 
> > ---
> >  arch/i386/math-emu/status_w.h |    7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > --- linux-2620-rc2.orig/arch/i386/math-emu/status_w.h
> > +++ linux-2620-rc2/arch/i386/math-emu/status_w.h
> > @@ -48,9 +48,10 @@
> >  
> >  #define status_word() \
> >    ((partial_status & ~SW_Top & 0xffff) | ((top << SW_Top_Shift) & SW_Top))
> > -#define setcc(cc) ({ \
> > -  partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
> > -  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); })
> > +#define setcc(cc) do { \
> > +	partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
> > +	partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); \
> > +} while (0)
> 
> Is there any reason you this shouldn't be an inline function?

That would be OK in theory, so I just tried it.

I don't get the same object files produced with an inline function
(for arch/i386/math-emu/fpu_etc.o), so I don't feel that it's quite
as safe without digging deeping into the .o file and its changes.

The 3 other .o files that use setcc() were the same when using
the inline patch version.

---
From: Randy Dunlap <randy.dunlap@oracle.com>

setcc() in math-emu is written as a gcc extension statement expression
macro that returns a value.  However, it's not used that way and it's
not needed like that, so just make it a do-while non-extension macro
so that we don't use an extension when it's not needed.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 arch/i386/math-emu/status_w.h        |    5 +++--

---
 arch/i386/math-emu/status_w.h |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- linux-2620-rc2.orig/arch/i386/math-emu/status_w.h
+++ linux-2620-rc2/arch/i386/math-emu/status_w.h
@@ -48,9 +48,11 @@
 
 #define status_word() \
   ((partial_status & ~SW_Top & 0xffff) | ((top << SW_Top_Shift) & SW_Top))
-#define setcc(cc) ({ \
-  partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3); \
-  partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3); })
+static inline void setcc(int cc)
+{
+	partial_status &= ~(SW_C0|SW_C1|SW_C2|SW_C3);
+	partial_status |= (cc) & (SW_C0|SW_C1|SW_C2|SW_C3);
+}
 
 #ifdef PECULIAR_486
    /* Default, this conveys no information, but an 80486 does it. */