Is it necessary to set a function with always_inline attribution when the function would work maybe once?

Question

I'm going through a library for the ILI9488 running by SPI and want to copy this library in my version.

In the beginning of the source file, there's a guard for the SPI_HAS_TRANSACTION that if the library would run in Arduino compiler then it would use the SPI functions, otherwise it should run from another platform SPI libraries.

Library link: jaretburkett / ILI9488

This is the part of code I have questions about:

// If the SPI library has transaction support, these functions
// establish settings and protect from interference from other
// libraries.  Otherwise, they simply do nothing.
#ifdef SPI_HAS_TRANSACTION
static inline void spi_begin(void) __attribute__((always_inline));
static inline void spi_begin(void) {
#if defined (ARDUINO_ARCH_ARC32)
  // max speed!
  SPI.beginTransaction(SPISettings(16000000, MSBFIRST, SPI_MODE0));
#else
    // max speed!
  SPI.beginTransaction(SPISettings(24000000, MSBFIRST, SPI_MODE0));
#endif
}
static inline void spi_end(void) __attribute__((always_inline));
static inline void spi_end(void) {
  SPI.endTransaction();
}
#else
#define spi_begin()
#define spi_end()
#endif

My questions:

1. why the programmer first declare the function in source file not in header file ?
2. why to use always_inline attribution for a function that's is most the time used once in program file and it's not likely to be used many times during operating time ?
3. what does this define means ARDUINO_ARCH_ARC32 and where to find it ? I searched in all files in arduino folders and didn't find it. Is it dedicated for fast boards or the usual 16MHz arduino boards ?

---

Edit:

I want to add another question for the same library.

This function content:

void ILI9488::spiwrite(uint8_t c) {
//Serial.print("0x"); Serial.print(c, HEX); Serial.print(", ");
if (hwSPI) {
#if defined (AVR)
  #ifndef SPI_HAS_TRANSACTION
    uint8_t backupSPCR = SPCR;
    SPCR = mySPCR;
  #endif
    SPDR = c;
    while(!(SPSR & _BV(SPIF)));
  #ifndef SPI_HAS_TRANSACTION
    SPCR = backupSPCR;
  #endif
#else
    SPI.transfer(c);
#endif
  } else {
#if defined(ESP8266) || defined (ARDUINO_ARCH_ARC32)
    for(uint8_t bit = 0x80; bit; bit >>= 1) {
      if(c & bit) {
    digitalWrite(_mosi, HIGH);
      } else {
    digitalWrite(_mosi, LOW);
      }
      digitalWrite(_sclk, HIGH);
      digitalWrite(_sclk, LOW);
    }
#else
    // Fast SPI bitbang swiped from LPD8806 library
    for(uint8_t bit = 0x80; bit; bit >>= 1) {
      if(c & bit) {
    //digitalWrite(_mosi, HIGH);
    mosiport |=  mosipinmask;
      } else {
    //digitalWrite(_mosi, LOW);
    mosiport &= ~mosipinmask;
      }
      //digitalWrite(_sclk, HIGH);
      clkport |=  clkpinmask;
      //digitalWrite(_sclk, LOW);
      clkport &= ~clkpinmask;
    }
#endif
  }
}

what I understood from this is that the programmer wants the library to be more versatile to support AVR chips programmer by another IDE than Arduino, also supports ESP8266 and anything else should be handled by softSPI, actually pretty smart coding I guess.

My question here, is does #if defined (__AVR__) checks for either AVR/Arduino in general and when specifies SPI_HAS_TRANSACTION means Arduino SPI library specifically ? right ?

Answer 1

1. The function is only used in the library, and not part of the "API" of that library. Kind of like a private function.

2. Both functions are both being called 14 times inside the source code of this library. But in the larger scheme of things, it doesn't add a lot. But why not add it and maybe save a few bytes, and a few clock cycles.

3. The ARDUINO_ARCH_ARC32 is for the Arduino 101 board (Intel Curie processor).

Answer 2

1. why the programmer first declare the function in source file not in header file ?

the declarion is for the inline attribute. the function should be used only in this cpp.

2. why to use always_inline attribution for a function that's is most the time used once in program file and it's not likely to be used many times during operating time ?

there are 14 occurrences of transaction start and it is called before every SPI communication commands set in the library. see my answer about beginTransaction

3. what does this define means ARDUINO_ARCH_ARC32 and where to find it ? I searched in all files in arduino folders and didn't find it. Is it dedicated for fast boards or the usual 16MHz arduino boards ?

the architecture define is generated by the builder from selected board's boards package folder name. ARC32 architecture is the Intel Arduino 101 boards package.

EDIT (answer to edited part of the question):

My question here, is does #if defined (__AVR__) checks for either AVR/Arduino in general and when specifies SPI_HAS_TRANSACTION means Arduino SPI library specifically ? right ?

__AVR__ is defined in avrlibc, the toolchain for AVR microcontrolers. Arduino uses avrlibc for AVR MCUs. It is not recommended to check __AVR__. The AVR family of MCU is now large with different peripherals so checking __AVR__ should be really restricted to CPU matters.

Every Arduino 'architecture' has its own SPI library. The transactions support was added later so not all implementations of the SPI library support transactions. To declare transactions availability in SPI library Arduino defined the SPI_HAS_TRANSACTION define.

Answer 3

Here I am only addressing question number 2. The others have already complete answers.

The choice to whether a function should be inlined or not is a speed/size trade-off. There is a cost to calling a function. On AVR, for example, the call instruction takes 4 CPU cycles, and ret takes another 4. Inlining the function removes this cost. On the other hand, you end up with as many copies of the function as places where it is inlined, and this in turn costs flash memory.

The compiler is usually smart enough to make wise decision regarding function inlining. In this case, it is not obvious what decision it would make on its own. On the one hand, the function is used multiple times in the code, which makes a good case against inlining it. On the other hand, the body of the function is tiny: only setting up some registers (the two parameters of SPIClass::beginTransaction(), including the implicit this) and jumping to another function, so the cost of inlining is small.

For this library, the programmer happened to have a strong opinion on this matter, and he though inlining was really the best option. Also, he did not thrust the compiler to make the good decision. This is the reason why he used the gcc extension:

__attribute__((always_inline))

And no, it is not necessary, it is just an optimization choice.