Skip to contentSkip to conversation

inconsistent asm macro

DISCUSSION Ed Martini 2 participants 2 messages Mar 30, 2005 – Jan 16, 2006 MIPS

lore
Share & tools
lore

Ed Martini

author
#0 lore raw

In include/asm-mips/interrupt.h, the definition for local_irq_restore is inconsistent in its use of .reorder/.noreorder assembler directives. Other asm macros in interrupt.h are wrapped with '.set push' and '.set pop'.

It doesn't seem to be a problem with the 2.96 mipsel-linux- assembler, but it caused me a problem with my 4.0-based toolchain. (As it was the local_irq_restore left the assembler in 'reorder' mode and a stack pointer post-inc was reordered out of the return delay slot where it belonged.) Luckily we have a sharp compiler guy who figured it out. Thanks.

As usual, there may be a reason for this, but it took me a whole day to find it, and I thought I'd point it out.

Ed Martini

$ diff -uN interrupt.h interrupt-new.h
--- interrupt.h 2005-03-29 17:35:02.922362384 -0800
+++ interrupt-new.h     2005-03-29 17:33:26.350770293 -0800
@@ -100,6 +100,7 @@

__asm__ (
".macro\tlocal_irq_restore flags\n\t"
+ ".set\tpush\n\t"

       ".set\tnoreorder\n\t"
       ".set\tnoat\n\t"
       "mfc0\t$1, $12\n\t"
@@ -109,8 +110,7 @@
       "or\t\\flags, $1\n\t"
       "mtc0\t\\flags, $12\n\t"
       "irq_disable_hazard\n\t"
-       ".set\tat\n\t"
-       ".set\treorder\n\t"
+       ".set\tpop\n\t"
       ".endm");
#1 lore raw
Quoted 11 lines · Ed Martini

In include/asm-mips/interrupt.h, the definition for local_irq_restore is inconsistent in its use of .reorder/.noreorder assembler directives. Other asm macros in interrupt.h are wrapped with '.set push' and '.set pop'.

It doesn't seem to be a problem with the 2.96 mipsel-linux- assembler, but it caused me a problem with my 4.0-based toolchain. (As it was the local_irq_restore left the assembler in 'reorder' mode and a stack pointer post-inc was reordered out of the return delay slot where it belonged.) Luckily we have a sharp compiler guy who figured it out. Thanks.

Quoted 2 lines

As usual, there may be a reason for this, but it took me a whole day to find it, and I thought I'd point it out.

Maciej, since you use gcc 4, can you please review this patch?

Quoted 24 lines

Ed Martini

$ diff -uN interrupt.h interrupt-new.h
interrupt-new.h 2005-03-29 17:33:26.350770293 -0800 renamed from interrupt.h 2005-03-29 17:35:02.922362384 -0800 +0 -0
@@ -100,6 +100,7 @@
 

__asm__ (
".macro\tlocal_irq_restore flags\n\t"

interrupt-new.h 2005-03-29 17:33:26.350770293 -0800 renamed from interrupt.h 2005-03-29 17:35:02.922362384 -0800 +2 -2
+       ".set\tpush\n\t"
       ".set\tnoreorder\n\t"
       ".set\tnoat\n\t"
       "mfc0\t$1, $12\n\t"
@@ -109,8 +110,7 @@
       "or\t\\flags, $1\n\t"
       "mtc0\t\\flags, $12\n\t"
       "irq_disable_hazard\n\t"
-       ".set\tat\n\t"
-       ".set\treorder\n\t"
+       ".set\tpop\n\t"
       ".endm");
 
 
Signature · 2 lines
Martin Michlmayr
http://www.cyrius.com/