1

I am trying to adopt a piece of code, where there is a declaration of a C routine; which mainly consists of an inline assembly code. However when I compile/link it, IN case i call the C-routine from main I get the assembly/linker error

> C:\Documents and Settings\ge\Skrivbord\LED strip\GITHUB code>avr-gcc -g -Os -mmcu=atmega8 -c ws2.c
C:\DOCUME~1\ge\LOKALA~1\Temp/ccVFmJez.s: Assembler messages:
C:\DOCUME~1\ge\LOKALA~1\Temp/ccVFmJez.s:136: Error: symbol `pr_byte' is already defined
C:\DOCUME~1\ge\LOKALA~1\Temp/ccVFmJez.s:146: Error: symbol `one_bit' is already defined
C:\DOCUME~1\ge\LOKALA~1\Temp/ccVFmJez.s:154: Error: symbol `both_low' is already defined
C:\DOCUME~1\ge\LOKALA~1\Temp/ccVFmJez.s:163: Error: symbol `nxt_byte' is already defined
C:\DOCUME~1\ge\LOKALA~1\Temp/ccVFmJez.s:169: Error: symbol `done' is already defined

where as if I comment out the call I do not get that error, code is as follows:

#include <avr/io.h>

#define F_CPU 8000000   // 8MHz


void update_line( const void *values, uint16_t array_size, uint8_t output_bit){


asm volatile(

" LD __tmp_reg__, %a[dataptr]+       \n"     
" LDI %[bits], 8                     \n"     // load no of bits in byte count

"pr_byte:                            \n" 
" LSL __tmp_reg__                    \n"     //  Load next bit into carry flag.
" OUT %[portout], %[upreg]           \n"     // Go high, start of bit-symbol,
" BRCS one_bit                       \n"     //  bit value is in carry flag
" NOP                                \n"
" OUT %[portout], %[downreg]         \n"     // go low for 0-bit-symbol (3clc) (carry low)

" NOP                                 \n"
" NOP                                 \n"
" NOP                                 \n"
"  rjmp both_low                     \n"     // 

"one_bit:                            \n"     //  

" NOP                                 \n"
" NOP                                 \n"
" NOP                                 \n"
" NOP                                 \n"
" NOP                                 \n"
" NOP                                 \n"     
" OUT %[portout], %[downreg]         \n"     // go low for the 1-bit-symbol               
"both_low:                           \n"     //  both low; time to initiate next bit 
" SUBI %[bits], 1                    \n"     //  bit countdown for byte
" BREQ nxt_byte                      \n"     // 

" NOP                                 \n"     // 5nop still bits left to process
" NOP                                 \n"
" NOP                                 \n"
" NOP                                 \n"
" NOP                                 \n"
" rjmp pr_byte                        \n"     // take next bit

// end of previous byte, pick new 
"nxt_byte:                           \n"     // 3used/10
" SBIW %[bytes], 1                   \n"     // byte countdown
" BREQ done                          \n" 
" LD __tmp_reg__, %a[dataptr]+       \n"     // Load next byte
" LDI %[bits], 7                     \n"     // load bit-in-byte count

// fill time before next bit-symbol, none left, kritical ?
"rjmp pr_byte                        \n"     // take next

"done:  NOP \n"   // program should end in low note, som contents will be displayed

: /* no output */
: /* inputs */
[dataptr] "e" (values),                  
[upreg]   "r" (set_obit_high),             
[downreg] "r" (set_obit_low),            
[bytes]   "w" (size),                      
[bits]    "d" (bitcount),                
[portout] "I" (_SFR_IO_ADDR(WS2811_PORT)) 

);
}  /* update_line() */

int main () {

uint8_t LED_rgb;
struct rgb LED_string[48];    // a continuous string of 3 bytes(rgb)
int LED_len = 48; 

//update_line( LED_string, 48*3, 1);  //<*************

} /*end main*/

`

so the interresting thing is there seem to have been generated labels for the "update_line" routine first when the routine is declared, and then ALSO when the routine is called ! so in some way it seems that the whole routine becomes "inline" (in the sense that its code is placed at the position of call itself). Im a bit at loss as both what is happening and as to what can be done (there seem to be possible to generate new labels using %= / but still why should the code be multiplied)

tnx Georg

  • 1
    I suspect that if you were to use -S to generate the assembler output (foo.S), you would see the problem. The S file will likely contain both the function definition for update_line, as well as the inlined version included into main(), both of which define the same symbols. You might be able to avoid the problem using "local labels" (or whatever implementation your assembler has), but `%=` might actually be better (not sure what happens if main calls update_line twice). However, let me take a minute to pitch NOT using inline asm (see [this](https://gcc.gnu.org/wiki/DontUseInlineAsm)). – David Wohlferd Dec 30 '16 at 22:19
  • [Canonical Q&A for how to deal with the duplicate label problem](https://stackoverflow.com/questions/3898435/labels-in-gcc-inline-assembly). Not a duplicate because this question is asking *why* the compiler can duplicate your asm statement in the first place. – Peter Cordes Mar 22 '18 at 03:18

2 Answers2

3

GCC provides a mechanism to counteract this in the form of %=.

According to GCC's documentation (under Special format strings):

‘%=’

Outputs a number that is unique to each instance of the asm statement in the entire compilation. This option is useful when creating local labels and referring to them multiple times in a single template that generates multiple assembler instructions.

Simply appending this to the names of your labels (including in instructions that reference the labels) should fix the duplication problem.

The compiler does this by (as described in the afforementioned quote) associating a number with each instance of 'pasted' assembly code and replacing %= with that number.

(I can verify this works because I've just had the exact same problem on a much smaller scale, which is how I ended up here in the first place.)

Pharap
  • 3,826
  • 5
  • 37
  • 51
1

void update_line(args){ body } is not just a declaration. It's a global definition that the compiler can't omit, even if it decides to inline it into every caller in this compilation unit (this .c file).

So the compiler has to emit a stand-alone definition of the function.

It also has to copy it verbatim into all places where it decides to inline the function body into callers. This is what it means to inline a function. If you don't want that, use __attribute__((noinline)). gcc doesn't "read" your asm, so it doesn't realize that there's a lot of code. I thinks of it as just a single instruction (that behaves as described by the constraints), so it looks like a small function that it should inline everywhere it's used.

If you'd used static void update_line, the compiler would know that this definition wasn't visible from outside the compilation unit, and leave out the stand-alone definition.

But it could still inline into two or more callers, so you should use %= as part of your label names. (Or numbered labels, and refer to them with 1f or 1b for forward/backward).

Or better, only use inline asm for the I/O instructions. Write the branching logic in C so you don't need a giant block of asm, and the compiler can optimize that code into the caller.

Or if you really want to write the whole function in asm (so you can use NOPs for delays I guess?), you could just write it as a stand-alone function that you call from C.

Peter Cordes
  • 328,167
  • 45
  • 605
  • 847
  • 1
    "it doesn't realize that there's a lot of code" - Not precisely true. See [this](https://gcc.gnu.org/onlinedocs/gcc/Size-of-an-asm.html). Whether it uses this information when making inline decisions, I don't know. But it does collect this information and use it, so it could. – David Wohlferd Dec 31 '16 at 01:27
  • Thanks; yes I have now used the %=; and yes, the NOP are for timing, –  Dec 31 '16 at 01:38
  • (couldnt edit my previous comment); I thought this asm code would go into the definition of the C-procedure, which I my view would be called from where-ever used, and not instancieated there; which might be just what you mention as "stand-alone function", but what should I then do to do a such ? thx (or where should i put attribute (noinline) ?) –  Dec 31 '16 at 01:46
  • @Georg: It does go into the definition of the C function. Apparently you're not understanding what inlining does. Look at this simple compiler output: https://godbolt.org/g/ewiMal for an example of with/without `noinline`. – Peter Cordes Dec 31 '16 at 01:52
  • Hi Peter; exactly; Im not understanding. to begin with; how did I know that I was doing "inlining", all I do is declaring a piece of code as "asm", thinking that it will go into the routine ! (I will check your ref to try to clarify this, but my take is that semantics is unclear) This now acts as the whole C-defined routine is an inline macro (at least in my langugae), and that gets elaboradted AS well where it is declared, strange) –  Dec 31 '16 at 15:03
  • 1
    @Georg: The C compiler is allowed to do whatever it wants, as long as the compiled program behaves the same. Inlining is an important and useful optimization (https://en.wikipedia.org/wiki/Inline_expansion), so optimizing compilers do it by default. You have to take special steps if you want to *stop* it from happening. The compiled result is similar to if you'd used a macro, but you have all the type safety and everything else of using a function. Since compilers are good at inlining, we don't have to use macros anymore to avoid hurting performance. – Peter Cordes Dec 31 '16 at 15:18