[SAMC] pointer code only works within a function

[Simon]

I am just testing out my first very basic libraries for the SAMC. So lets toggle a pin. I started with just setting and resetting the pin and noticed that my little program required 20.5 clock cycles so i assume depending on which way the if/else goes 20 or 21 cycles. So I though right lets try toggle to see how much faster it is. Not that much at 18 cycles. So I thought maybe I should take my code from the function and just put that in to so if it runs quicker. But then it stopped working. If I call my function it works, but if I just use the functions code nothing.

So the function code:

Code: [Select]

void pin_tgl(uint8_t port, uint8_t pin ){
( * ( uint32_t * )( PORT_BASE + PORT_PIN_TOGGLE + port * PORT_OFFSET ) ) = 0x1 << pin ;
}

So

pin_tgl( 1, 8 );   

gets me a 111kHz square wave.

( * ( uint32_t * )( PORT_BASE + PORT_PIN_TOGGLE + 1 * PORT_OFFSET ) ) = 0x1 << 8;

gets me nothing
even

( *  (uint32_t * )( 0x4100009C)  ) = 0x00000100 ;

got nothing.

So what is going on? why will the code only work when it is wrapped in a function?

[ataradov]

Try "( *  (volatile uint32_t * )( 0x4100009C)  ) = 0x00000100 ;". It might be a compiler optimization.

Also, looking at the disassembly is a great debugging tool in cases like this.

I usually add a bunch of nop instructions to be able to locate the piece of code easily.

Code: [Select]

asm("nop");
asm("nop");
asm("nop");
asm("nop");
( *  (uint32_t * )( 0x4100009C)  ) = 0x00000100 ;
asm("nop");
asm("nop");
asm("nop");
asm("nop");

Then run "arm-none-eabi-objdump -d file.elf > disasm.txt".

And then look for those nops and see what happened to the code in between.

[Simon]

Yes that works, but volatile is for a variable that can be changed by another part of a program like an interrupt. what is the relevance here?

Now running 400kHz so 5 clock cycles, the function call and return is taking 13 cycles

[Simon]

should I put volatile on any pointer to a register?

[ataradov]

Yes, you should. Otherwise compiler assumes that it is a normal memory location. And if  you write into a variable, but never read back, the write is not necessary and can be optimized.

[Simon]

right. Thanks

[andersm]

but volatile is for a variable that can be changed by another part of a program like an interrupt.

No. The volatile qualifier was added to the language as a tool for accessing hardware registers. The formal definition is something like "any expression referring to [a volatile-qualified object] shall be evaluated strictly according to the rules of the abstract machine". In practice it is a method of telling the compiler that touching some things has side-effects which it can't deduce from the code, and thus isn't allowed to eg. perform some types of optimizations. While volatile is often used with variables shared between interrupts and user mode code, it is in fact not the right tool for that, since it doesn't protect you against race conditions. Instead, you should use atomic types, provided your compiler supports them.

[mikerj]

While volatile is often used with variables shared between interrupts and user mode code, it is in fact not the right tool for that, since it doesn't protect you against race conditions. Instead, you should use atomic types, provided your compiler supports them.

Volatile should be used in addition to atomic access methods in this case.

[Siwastaja]

Also add the same volatile to the function version. It's now working by pure luck. It's also slow for the same reason: it's not defined inline.

Define it as "static inline", and it becomes as quick as your direct code copypasta version - and likely also stops working.

[Simon]

Define it as "static inline", and it becomes as quick as your direct code copypasta version - and likely also stops working.

You mean it would stop working if i did not put volatile into the static function. So if I use static it will basically just pun the function content in and not treat it as a function call and return?

[Simon]

static inline void pin_tgl(uint8_t port, uint8_t pin ){
   ( * ( volatile uint32_t * )( PORT_BASE + PORT_PIN_TOGGLE + port * PORT_OFFSET ) ) = 0x1 << pin ;
}

produces the error: static declaration of 'pin_tgl' follows non-static declaration   

[Simon]

do i have to take out the "void"?

[Simon]

it can't be static because it takes arguments surely?

[Siwastaja]

it can't be static because it takes arguments surely?

Absolutely nothing to do with arguments. You know, you can look these things up on Google instead of guessing.

Static function defines whether you want to expose the function to other modules - i.e., other, separately compiled C files. It's a qualifier, like inline, so you use the return type as well:

static inline void function(int argument)
{

}


If you are making a library, for example, you want to have the interface functions without static qualifier. The "internals", i.e. functions that your module calls (for code readability / reuse within that module) can and should be all static.

Without static, compiler needs to expose the function for linker, and is forced to assume anyone can call the function from anywhere, so it can't optimize much. With static, only that file uses the function, so compiler can do whatever it wants.

A beginner often does small projects in one file. In this case, all functions should be made static. Yes, beginners also forget to do that, resulting in larger and slower code.

[Simon]

i was not the one wanting to make it static. Apparently it was going to make it faster.

[AG6QR]

static inline void pin_tgl(uint8_t port, uint8_t pin ){
   ( * ( volatile uint32_t * )( PORT_BASE + PORT_PIN_TOGGLE + port * PORT_OFFSET ) ) = 0x1 << pin ;
}

produces the error: static declaration of 'pin_tgl' follows non-static declaration

That suggests the problem is that you have a non-static declaration of the function before your static declaration.  Did you make a function prototype anywhere?  In a header file?

If so, make sure the declaration matches the definition.

[Simon]

Yes, did that but then as explained it means the function can only be used in that file which is not main.c so it failed to work as the definition of the function was missing.

[donotdespisethesnake]

If you make a function inline, and want to use it in a different file, put the whole function definition into the header file, and remove it from the .c file.

[Simon]

ooh yea baby, function call with nearly 4x the speed.

[ataradov]

And I once again will strongly advice to looks at the disassmbly. It has multiple benefits:
1. You will become more familiar with the ARM assembly.
2. You will be more familiar with the compiler behaviour in general.
3. You will actually know if compiler does what you expect it to do.

[Simon]

how do I get the assembly from AS7? I am not overly interested as i have enough on my plate but was curious and in time yes i should look at it.

[ataradov]

There is a disassembly view, but it is bad. I would not look for anything other tan actual step-by-step debugging.

To get a real disassembly use  "arm-none-eabi-objdump -d file.elf > disasm.txt". You can add this into your post-build events in the AS project settings and it will automatically generate the text file every time you build the project.

[Nerull]

inline often does nothing on modern compilers. The compiler can and will decide for itself if a function should be inlined, and it will inline functions you didn't ask it to and ignore your requests to inline functions it doesn't think are a good idea. It may even eliminate functions entirely.

For example, here's this simple code compiled on clang 8 with -O2:

Code: [Select]

static int square(int num) {
    return num * num;
}

int main() {
    return square(5);
}

And here's the intel-format generated ASM:

Code: [Select]

        mov     eax, 25
        ret

That's it.

It does a similar thing without 'static', but it keeps the code for the square function around, just never calls it. Link time optimization would likely remove that as well. Sticking -flto on the command line confirms that 'static' makes no difference in this case.

[Simon]

yes because the function did not take any runtime input. My functions do although they are predetermined and maybe if i used a lot of them it would stop inlining and call it to save space. i am not overly bothered but if inlining the function makes it more likely to to be treated as such and speed up the program great. If it can't then so be it.

[andersm]

yes because the function did not take any runtime input.

Having constant arguments allows the compiler to evaluate the function call at compile time, but it's not a limit for inlining. Rather, the compiler will use some heuristic to determine if it should inline the function call or not. GCC for example will almost always inline a function if the overhead of calling it is larger than the function body itself.

[westfw]

Code: [Select]

void pin_tgl(uint8_t port, uint8_t pin ){
( * ( uint32_t * )( PORT_BASE + PORT_PIN_TOGGLE + port * PORT_OFFSET ) ) = 0x1 << pin ;
}

That looks like a pretty convoluted and error-prone way of saying:

Code: [Select]

void pin_tgl(uint8_t port, uint8_t pin ){
  (PORT->Group[port].OUTTGL.reg) = 0x1 << pin ;
}(and yes, that compiles to pretty optimal code, at least on a SAMD21.  (Which should be the same WRT general purpose IO as a SAMC20))(and also yes, whoever was at Atmel and come up with the name "PORT" for the start of the group of ports, and "Group" for each individual port, should be punished.  Sigh.)
I think in the more recent Microchip incarnations of CMSIS peripheral definitions for SAM*, you need to say:

Code: [Select]

void pin_tgl(uint8_t port, uint8_t pin ){
  (PORTS[port].PORT_OUTTGL) = 0x1 << pin ;
}Which is a little better, ASIDE FROM COMPLETELY REDOING EVERYTHING.  Double Sigh.

[ataradov]

Simon likes to redo things. There is absolutely no reason for that, but I don't think we will change his mind any time soon.

[sokoloff]

When you’re first learning, that’s often a good thing when the purpose is to aid understanding.

[Simon]

Code: [Select]

void pin_tgl(uint8_t port, uint8_t pin ){
( * ( uint32_t * )( PORT_BASE + PORT_PIN_TOGGLE + port * PORT_OFFSET ) ) = 0x1 << pin ;
}

That looks like a pretty convoluted and error-prone way of saying:

Code: [Select]

void pin_tgl(uint8_t port, uint8_t pin ){
  (PORT->Group[port].OUTTGL.reg) = 0x1 << pin ;
}(and yes, that compiles to pretty optimal code, at least on a SAMD21.  (Which should be the same WRT general purpose IO as a SAMC20))(and also yes, whoever was at Atmel and come up with the name "PORT" for the start of the group of ports, and "Group" for each individual port, should be punished.  Sigh.)
I think in the more recent Microchip incarnations of CMSIS peripheral definitions for SAM*, you need to say:

Code: [Select]

void pin_tgl(uint8_t port, uint8_t pin ){
  (PORTS[port].PORT_OUTTGL) = 0x1 << pin ;
}Which is a little better, ASIDE FROM COMPLETELY REDOING EVERYTHING.  Double Sigh.

i got fed up with trawling through the header files trying to work out what to call stuff. This way it's self explanatory and i like self explanatory code as later I know what the heck I did. Given the 5 cycle run time in a while loop I'm sure the compiler does not give a fig how it's written as long as it's valid.

[Simon]

Simon likes to redo things. There is absolutely no reason for that, but I don't think we will change his mind any time soon.

Nope, I have a short memory (probably ADD) so doing it myself helps it stick in memory to learn, means that what I have to remember is more like what I would naturally want to write and the code self documents, something the vendors rarely do as discussed many times. I try to write code that self explains itself as much as possible so that the day i have to figure out what the hell i did it's all laid bare there for me. I swapped from raw register name addresses to defining them so that at least on a same as but differently addressed chip i can simply change the address definitions.

[Simon]

And of course in doing this i have learnt more about C the easier way, by doing it. i have come to appreciate the time it take to call a function and the fact that volatile was supposed to be used in any pointer to register is something I have never found explained.

[Simon]

looking at the clock controller. I don't think it's as complicated as people think it is. What's complicated is the way they describe it repeating themselves and using names in the text to talk about blocks in a diagram that instead have different names.

Several clock sources are available. you connect them to a pre-scaler or clock generator that in turn is connected to peripheral clock inputs.

[andersm]

While volatile is often used with variables shared between interrupts and user mode code, it is in fact not the right tool for that, since it doesn't protect you against race conditions. Instead, you should use atomic types, provided your compiler supports them.

Volatile should be used in addition to atomic access methods in this case.

Correct me if I'm wrong, but with sequentially consistent memory ordering, the volatile would be redundant.

[ataradov]

Correct me if I'm wrong, but with sequentially consistent memory ordering, the volatile would be redundant.

No. Anything you change or access from main code and the interrupt has to be volatile, otherwise compiler is free to optimize either of those things out.

This has to be done in addition to any atomicity concerns.

[Simon]

You better get microchip to update this page: https://microchipdeveloper.com/32arm:sam-bare-metal-c-programming not one mention of volatile.

[sokoloff]

You better get microchip to update this page: https://microchipdeveloper.com/32arm:sam-bare-metal-c-programming not one mention of volatile.

Won't be needed. Microchip already defines the access pointers to be volatile correctly:
https://github.com/avrxml/asf/blob/master/sam0/utils/cmsis/samd21/include/samd21j18a.h

RWReg is defined as:

Code: [Select]

typedef volatile       uint32_t RwReg;   /**< Read-Write 32-bit register (volatile unsigned int) */

[donotdespisethesnake]

You better get microchip to update this page: https://microchipdeveloper.com/32arm:sam-bare-metal-c-programming not one mention of volatile.

Yeah, that page is not well written. Probably written by technical authors with not much real C experience. Makes some very dubious claims about C, and completely skips over the importance of volatile. Sure they are buried in the header definitions, but a newcomer is not going to look there or understand the significance.

[westfw]

You better get microchip to update this page: https://microchipdeveloper.com/32arm:sam-bare-metal-c-programming not one mention of volatile.

For the "instance" and "component" .h files that they talk about, the "volatile" keywords are buried in the (not shown) definitions of RwReg, __IO, and etc.

But it does seem bad that they didn't mention it the section where they talk about defining them manually.
PIC32 doesn't have a corresponding section :-(


Who is this "you" that you talk about ?  :-)

[nigelwright7557]

but volatile is for a variable that can be changed by another part of a program like an interrupt.

No. The volatile qualifier was added to the language as a tool for accessing hardware registers. The formal definition is something like "any expression referring to [a volatile-qualified object] shall be evaluated strictly according to the rules of the abstract machine". In practice it is a method of telling the compiler that touching some things has side-effects which it can't deduce from the code, and thus isn't allowed to eg. perform some types of optimizations. While volatile is often used with variables shared between interrupts and user mode code, it is in fact not the right tool for that, since it doesn't protect you against race conditions. Instead, you should use atomic types, provided your compiler supports them.

Excellent reply, very comprehensive.

[Simon]

You better get microchip to update this page: https://microchipdeveloper.com/32arm:sam-bare-metal-c-programming not one mention of volatile.

Won't be needed. Microchip already defines the access pointers to be volatile correctly:
https://github.com/avrxml/asf/blob/master/sam0/utils/cmsis/samd21/include/samd21j18a.h

RWReg is defined as:

Code: [Select]

typedef volatile       uint32_t RwReg;   /**< Read-Write 32-bit register (volatile unsigned int) */

Yes but that page was supposed to be explaining the nuts and bolts of register addressing. At some point in there it actually explains what no book I have does which is how you use a pointer to address a register and put a value in it. Tha barest of code is in that page but without the volatile.

[Jeroen3]

Their example is confusing.

To define a pointer to this specific memory address we can do the following:

And then proceed to name it PORT0_DIR_ptr instead of foo, as all examples do.

There was an attempt, unfortunately the code lies.

Correct would be, for addressing a register:

Code: [Select]

volatile unsigned int * const prt_register = (unsigned int *)0x41004400;Where the const makes the pointer itself immutable.
https://cdecl.org/?q=volatile+unsigned+int+*+const+prt_register

[nigelwright7557]

Yes that works, but volatile is for a variable that can be changed by another part of a program like an interrupt. what is the relevance here?

Volatile is relative to interrupt/main program data but also SFR's.
Anything that can change data without the cache knowing.

There can also be a problem with DMA and the cache where DMA-ing data doesnt update the cache on some micro's.

[westfw]

Anything that can change data without the cache knowing.

Anything that can change data without the COMPILER knowing.AFAIK, "volatile" won't help at all with cache management; that's a whole different can of worms!

[ejeffrey]

Anything that can change data without the cache knowing.

Anything that can change data without the COMPILER knowing.AFAIK, "volatile" won't help at all with cache management; that's a whole different can of worms!

That is correct.  Volatile prevents the compiler from combining, eliminating, duplicating, or reordering memory read/write CPU instructions.  That is all it does.  It doesn't guarantee atomiticity, it doesn't deal with any low level architectural details like cache and memory mapping units, and it doesn't guarantee that any other processor or peripheral in the system will see those access in any particular order.  If you are expecting any of those things (even if you don't realize it) volatile is not what you want, or at best part of what you want.

[Simon]

So what does one do? Does having a cache cause all of this trouble?

[ataradov]

So what does one do?

For what specifically? For memory mapped registers? Declare them volatile.

Does having a cache cause all of this trouble?

Cache causes a whole new level of issues, but volatile has nothing to do with them. Cache is dealt with cache maintenance operations.

But given the scope of the cache in the SAM C, you don't need to worry about it at all.

[Siwastaja]

Since a few microcontrollers with caches have come to market, discussions (and misconceptions) about caches blew out of proportion, they are discussed here almost every day.

Once again I repeat, most microcontrollers have no caches; and most of those few that do, boot all caches disabled by default, or at least you are very likely to know if you use one which has caches enabled.

If you are unsure, you can always check, and then just turn it off. In most microcontroller cases, cache won't make that much performance difference because you tend to have everything in on-chip SRAM anyway, and run code from the wide pre-fetched on-chip flash, or even better, RAM.

[westfw]

I repeat, most microcontrollers have no caches;

Many microcontrollers have some sort of cache in front of program memory.   ESP8266, ESP32, SAMD5x have real caches, which is how they manage to run adequately XIP Serial flash.  Fortunately, program memory isn't usually volatile.  Data cache is less common, but it's catching on.

Caching will likely be disabled on peripheral registers, in hardware.  Usually this will be on a per memory-region basis.  For example, The ARMv7m specification defines a "Device" memory type attribute, while only "normal" memory is cacheable (and even then it can have several types of cacheability.)   See Section A3.5.2 Summary of ARMv7 memory attributes
(Note that some caches may not be using the ARM cache design, though.)

The biggest problem with data caching is likely to show up when you're using peripherals with DMA.

[Simon]

The only reason i can see for cache is fetching program instructions form ROM. even on the "simple" SAMD there are  wait states for running at 48MHz so does that mean that the ROM can supply instructions at 16MHz, what does the CPU do during the ROM wait states?

[ataradov]

what does the CPU do during the ROM wait states?

Nothing. Just waits for the instructions to arrive. That's why you need caches and accelerators of all sorts.

In reality majority of Thumb-2 instructions are 16-bits, and Cortex-M0+ always fetches 32-bit word, so in most cases it fetches 2 instructions at the same time.

But generally yes, things get worse as clock frequency goes up, since flash is slow.

[Simon]

So the SAMD does not explicitly have cache but sort of does something to try and compensate. Presumably RAM is at the same speed.

[ataradov]

So the SAMD does not explicitly have cache but sort of does something to try and compensate. Presumably RAM is at the same speed.

It does nothing. Executing from flash is just slower. RAM can fetch a word every cycle, so it is as fast as the core can go.

You can execute from RAM, of course, but you will obviously lose that RAM. And in may cases execution speed does not matter, but RAM does.

You can put just the critical stuff in RAM, like vector table and vector handlers, for example.

[Simon]

Well i suppose if yoau read an instruction that then require access to variables in RAM when you want the ROM again it's ready.

[Siwastaja]

Many "mid-range" chips use quite wide flash words for their internal flash, like 64 or 128 bits, meaning they fetch multiple instructions at once. Usually with these mid-range parts, the flash speed is like 1/4 of the CPU frequency, so if you fetch 4 instructions at the time, there are no wait states.... in theory, if there are no jumps in the code.

This may or may not be called some kind of "accelerator", or "prefetch". It's like a very trivial and small case of a "cache".

In practice with mid-range STM32s (F3, F4) with such prefetch, I tend to see approx. 30% penalty from running from flash directly, compared to running from core-coupled instruction RAM region, not too bad usually.

Caches kind of suck in the microcontroller world, because often you want repeatability and predictability. Worst case performance, instead of average performance. Providing separate "scratchpad" RAM areas, i.e., separate RAM sections with their own interface, directly on the side of the CPU core, does much better job, so many MCUs provide those instead, or in addition, to caches.

In any case, instruction cache is not that bad, because typically you don't change the code on the fly, i.e., code is read only, so there are no consistency issues, cached data never goes bad.

Data caching comes with a bunch of problems, but data caching is still rare in the microcontroller world. I would more likely see larger number of core-coupled memories so you can put all timing-critical data there, and know it's always accessed in 1 cycle.

[andersm]

Data caching comes with a bunch of problems, but data caching is still rare in the microcontroller world.

You start seeing it when clock frequencies reach ca. 200MHz, like Cortex-M7 and PIC32MZ devices. When you have fast CPU cores coupled with fast peripherals, all competing for access to RAM, caches are almost a necessity.

[ataradov]

Well i suppose if yoau read an instruction that then require access to variables in RAM when you want the ROM again it's ready.

Cortex-M0+ has a single AHB-Lite interface. The code and data is fetched over the same interface anyway, so location of the code and data does not make a difference, it all will compete for the same interface.

On Cortex-M7, for example, there are two separate AHB interfaces. And in most (probably all) implementations they go to a multi-layer bus matrix. And typically SRAM has multiple ports too. So data and instructions can be fetched at the same exact time.

[Simon]

Yes but if the non volatile memory  has to wait does this affect RAM as well or is that all memory has to wait 2 cycles at 48MHz ?

[ataradov]

Yes but if the non volatile memory  has to wait does this affect RAM as well or is that all memory has to wait 2 cycles at 48MHz ?

RAM is always single cycle (no wait). So yes, executing entirely from RAM is faster in general.

[Simon]

Yes but if you lead an instruction from ROM that sais "go get some RAM data" that is one cycle getting the data from RAM that the ROM has waited so effectively there are some gains over 16MHz CPU frequency but you can't load more than 32Mips

[ataradov]

I'm not sure what you mean. The AHB-lite bus is sequential. When it waits for the flash, there can be no other loads. The entire bus is stalled.

[Simon]

Oh, but what if it does not need flash data next. If i have an instruction that says load a value from RAM can I got get that data from RAM on the next cycle as i have my instructions on what to do? The ROM can sit there an not be accessible while the CPU is reading/writing from something else.

[ataradov]

In general, data fetches take precedence over the instruction fetches. So if there are two new conflicting access requests, then data will get the bus.

But specific ordering of things is hard to predict exactly without going through the exact pipeline. A situation where RAM has to wait for flash is still possible.

Lets say on a certain cycle a prefetch unit issued a read from flash. This read will take multiple cycles. On the next cycle core is ready to read the data. Well, it won't be able to do that. Since the bus is locked waiting for the flash.

But gain, in real life performance impact of this is so negligible that it is not worth thinking about.

[Simon]

Yes that makes sense. I am not too worried about peformance. Moving to 32 bit over 8 bit is already a massive speed boost given that I can natively work on 16 or 32 bit variables. I don't know what the overhead for an 8 bit cpu carrying out 16 bit math is but I bet it is something in the order of slowing execution to a quarter.