ISRs, inline assembler and register assignment

Go To Last Post
10 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi everyone,

I have two globals directly assigned to a register and optimized an ISR with inline assembler. gcc always moves those the two globals into other registers in the setup. If I use the constraint "r" they are moved into r24 and r25, and if I use constraint "l" into r14 and r15.

Smart as I am, I assigned my globals to r14 and r15 (originally r2,r3), but gcc was smarter:

__vector_3:
/* prologue: frame size=0 */
/* prologue: naked */
/* prologue end (size=0) */
	lds r15,r15
	lds r14,r14
/* #APP */
      ...
/* #NOAPP */
	sts r15,r15
/* epilogue: frame size=0 */
/* epilogue: naked */
/* epilogue end (size=0) */
/* function void __vector_3() size 28 (28) */
	.size	__vector_3, .-__vector_3

It might be that the assembler optimizes the self assignments away, I just thought it's too funny.

I guess I will have to use the registers directly in the inline assembler and maintain the register assignment in two differnt places.

This is the code fragment in question:

#include 

static uint8_t mstate_ asm("r14");
static uint8_t mask_   asm("r15") = 0xFF;

/* copied from  and added 'naked' */
#ifdef __cplusplus
#define NAKED_ISR(vector)					\
extern "C" void vector(void);				\
void vector (void) __attribute__ ((signal,naked));		\
void vector (void)
#else
#define NAKED_ISR(vector)					\
void vector (void) __attribute__ ((signal,naked));		\
void vector (void)
#endif

#if 0
ISR( TIMER2_COMP_vect) {
  PORTD = mask_ & mstate_;
  mask_ = ~mask_;
}
#else
NAKED_ISR( TIMER2_COMP_vect) {
  asm volatile (
             "push __tmp_reg__"
      "\n\t" "in __tmp_reg__, __SREG__"
      "\n\t" "push __tmp_reg__"
      "\n\t" "lds __tmp_reg__, %[mask]"
      "\n\t" "and %[mask], %[mval]"
      "\n\t" "out %[mreg], __tmp_reg__"
      "\n\t" "com %[mask]"
      "\n\t" "pop __tmp_reg__"
      "\n\t" "out __SREG__, __tmp_reg__"
      "\n\t" "pop __tmp_reg__"
      "\n\t" "reti"
      :[mask] "=l" (mask_)
      :       "0"  (mask_)
      ,[mval] "l"  (mstate_)
      ,[mreg] "I"  (_SFR_IO_ADDR( PORTD))
      );
}
#endif

Using avr-gcc 4.0.2 on Debian:

avr-gcc -W -Wall -pipe  -mmcu=atmega8 -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -Os -Wall -S -o main.s main.c

Markus

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

General advise: if you end up tweaking entire functions
into inline assembler, stop right there. Open a new
file, use a filename suffix of .S (capital letter S), and
write straight in assembly there. The result will be way
better readable, and it will safe you a lot of time.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dl8dtl wrote:
General advise: ...

I don't agree with that on a general basis.

But even if I would, the point was that the compiler generates a self assignment, with an instruction that is highly questionable:

lds r15,r15

By all means, if it has to do a self assignment it should use

mov r15,r15

Since the behaviour is not described in the instruction set it is also not clear if the result of this operation is even defined (independent of lds and mov).

I have seen worse hoppala's from compilers (free and non free) and each time I stumble across one I have to chuckle.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, your code is fairly pointless. You are reserving a register
globally for a variable, and then try to pass it down through
the inline assembler anyway. That doesn't make any sense.
As you *know* you've already reserved that register globally,
you could use it immediately in your code.

Still, it's my faint believe hammering these few lines of assembly
into a separate file would improve readability by about an
order of magnitude.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dl8dtl wrote:
Well, your code is fairly pointless.

Out for a flame war, or just a crappy day? I never said that the globals are only accessed in that piece of inline assembler (which by the way has several bugs).

It is really bad coding practice to define the same thing in two different places, so writing "r2" in the assembler (inline or not) is out of the question. I could define a string and use that for the global declaration and in the assembler, but that makes for really bad readability (which seems to be your only concern, ignoring the issue at hand).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
static uint8_t mstate_ asm("r14");

But this doesn't declare a register variable, it declares a static
(SRAM) variable whose name is "r14". I don't know whether the
LDS runs afoul of gas's naming restrictions, but in any case I
think you told the compiler to ignore them by inserting the "asm()".

I guess I'm not quite clear what you expected to happen.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

From the avr-libc FAQ:

Quote:
How to permanently bind a variable to a register?

This can be done with

register unsigned char counter asm("r3");

See C Names Used in Assembler Code for more details.

Going through the assembler it seems to do the right thing. gcc doens't assign anything else to the register and each access to the globel accesses that register.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
register unsigned char counter asm("r3"); 

But that's not what you're doing.

static uint8_t mstate_ asm("r14");

Declares an SRAM variable (with a funny name), not a register variable.

From s. 5.37 in the GCC 3.4.6 manual:

Quote:
It is up to you to make sure that the assembler names you choose
do not conflict with any other assembler symbols. Also, you must not use
a register name; that would produce completely invalid assembler code.

I don't know whether gas does it, but there's no intrinsic reason the Rxx names
need to be reserved; as I recall the (V1) Atmel assembler didn't reserve them.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks, that was the problem. Done right, gcc really does use the globals in place.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

> I don't know whether gas does it, but there's no intrinsic reason the Rxx names
> need to be reserved;

They aren't reserved. The assembler figures the argument type out from the
opcode. Thus, you can also omit the "r" for registers at all.

>> Well, your code is fairly pointless.

> Out for a flame war, or just a crappy day?

Neither, just my opinion.

> I never said that the globals are only accessed in that
> piece of inline assembler...

Well, then perhaps better post a more complete
description in the first place.

Still, I think avoiding the entire inline asm mess is the
best way to go. You could use #define's to ensure
the same registers are used in C and asm code for the
global regs.

But of course, it's up to you. GCC's inline assembler is
really great, but it has a tendency to cause you to write
write-only code.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.