Port in PSTR macro changes from arduino core

[earlephilhower]

esp8266/Arduino#6565

[mikee47]

As the register keyword is deprecated and, in C++17, effectively banned, perhaps it should be removed? Unless you're aware of a specific use for it within the xtensa toolchain?

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4340

EDIT: Although the keyword still appears in the C standards I suspect it doesn't really do anything any more. But I could be wrong!

https://stackoverflow.com/questions/37456494/why-was-the-register-keyword-created

[earlephilhower]

No, I think you're right. Or, at least, C++17 makes it a warning deprecated and it breaks CI. I remember having to fix this in our Arduino core.

Although I'm not exactly sure what register you're referring to. Can you provide a link?

I'll also make asm->asm or whatever is the "right" way to make that happen, as you commented in the Sming repo IIRC.

[mikee47]

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html

The asm keyword is a GNU extension. When writing code that can be compiled with -ansi and the various -std options, use __asm__ instead of asm (see Alternate Keywords).

[mikee47]

Although I'm not exactly sure what register you're referring to. Can you provide a link?

It's used extensively within Arduino, but there are two instances within pgmspace.h in pgm_read_byte_inlined() and pgm_read_word_inlined():

static inline uint8_t pgm_read_byte_inlined(const void* addr) {
  register uint32_t res;
  pgm_read_with_offset(addr, res);
  return (uint8_t) res;     /* This masks the lower byte from the returned word */
}

[mikee47]

I've just attempted a Linux compile with optimisations disabled, and got this:

In file included from /home/mike/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/libc/include/sys/pgmspace.h:43,
                 from /home/mike/sandboxes/sming-dev/Sming/Wiring/FakePgmSpace.h:14,
                 from /home/mike/sandboxes/sming-dev/Sming/Wiring/FakePgmSpace.cpp:11:
/opt/esp-quick-toolchain/xtensa-lx106-elf/xtensa-lx106-elf/include/sys/pgmspace.h: In function 'uint32_t pgm_read_dword_unaligned(const void*)':
/opt/esp-quick-toolchain/xtensa-lx106-elf/xtensa-lx106-elf/include/sys/pgmspace.h:97:1: error: a15 cannot be used in asm here
   97 | }
      | ^

It's objecting to the A15 clobber, if I remove that it's fine.

(Note: I only ran this test at -O0 to check if libstdc++ got implicated, but thought I should mention it.)

[earlephilhower]

Got it on the format changes, thanks. I guess since I didn't include the right pgmspace.h they didn't pop up for me. :)

For a15 thing is probably a clobber list problem for me. I'll take a look at it. We never use -O0 because code blow up so bad, but I can see how it would make GDB much nicer...

[mikee47]

@earlephilhower Did you forget to change asm -> __asm__ as per #3 (comment) ?

[earlephilhower]

I did try the code with gcc9.2 and std=c++17 and -O0 in arduino with no errors. Are you sure about having an issue with asm(xxxx) as opposed to the 1-line asm ?

[mikee47]

This is the error I get:

/home/mike/sandboxes/esp-quick-toolchain/xtensa-lx106-elf.x86_64/xtensa-lx106-elf/include/sys/pgmspace.h:72:3: error: expected ')' before ':' token
   72 |   pgm_read_with_offset(addr, res);
      |   ^~~~~~~~~~~~~~~~~~~~

[earlephilhower]

Can you give a small .C file I can compile locally, and any GCC flags? I'm not seeing that (and it seems to be building in the Arduino CI OK, too)...

[mikee47]

#include <sys/pgmspace.h>

int main()
{
  auto c = pgm_read_byte(PSTR("abc"));
  return 0;
}

Compile with -std=c11 (or c99, etc) and it fails.

[mikee47]

If you build with g++ it doesn't mind... suspect that's an oversight in the compiler.

[mikee47]

Forgot to say, use gcc and it fails.

[mikee47]

This is interesting: https://stackoverflow.com/questions/13870489/is-inline-asm-part-of-the-ansi-c-standard. I don't have a problem working around this issue if it causes you problems. It only crops up compiling SPIFFS - we use the original source but I note arduino has pulled it in and converted to .cpp which is why this issue never comes up.

[earlephilhower]

I don't mind adding underscores, but I really can't repro this on my own setup even with the example you gave. I'm running from inside the esp-quick-toolchain dir and here's what I get. I think there's something else going on is my worry:

earle@server:~/src/esp-quick-toolchain$ cat a.c
#include <sys/pgmspace.h>

int main()
{
  auto c = pgm_read_byte(PSTR("abc"));
  return 0;
}

earle@server:~/src/esp-quick-toolchain$ cp a.c a.cpp
earle@server:~/src/esp-quick-toolchain$ ./xtensa-lx106-elf.x86_64/bin/xtensa-lx106-elf-g++ -c --std=c++11 a.cpp
earle@server:~/src/esp-quick-toolchain$ ./xtensa-lx106-elf.x86_64/bin/xtensa-lx106-elf-g++ -c --std=c++11 a.c
earle@server:~/src/esp-quick-toolchain$ ./xtensa-lx106-elf.x86_64/bin/xtensa-lx106-elf-g++ -c --std=c++17 a.c
earle@server:~/src/esp-quick-toolchain$ ./xtensa-lx106-elf.x86_64/bin/xtensa-lx106-elf-g++ -c --std=c++17 a.cpp
earle@server:~/src/esp-quick-toolchain$ ./xtensa-lx106-elf.x86_64/bin/xtensa-lx106-elf-gcc -c --std=c++17 a.cpp
earle@server:~/src/esp-quick-toolchain$ ./xtensa-lx106-elf.x86_64/bin/xtensa-lx106-elf-gcc -c --std=c++17 a.c
cc1: warning: command line option '-std=c++17' is valid for C++/ObjC++ but not for C
a.c: In function 'main':
a.c:5:8: warning: type defaults to 'int' in declaration of 'c' [-Wimplicit-int]
    5 |   auto c = pgm_read_byte(PSTR("abc"));
      |        ^
earle@server:~/src/esp-quick-toolchain$ 

[earlephilhower]

To be clear, this is the latest -gnu4 build (with all the PRs included). I just got home and will do the upload now.

[mikee47]

Ignore the auto thing, I'm too used to working in C++. See my addendum about this only failing with gcc, i.e:

mike@UbuntuME:~/sandboxes/sming-dev/tests/asmtest$ /opt/esp-quick-toolchain/xtensa-lx106-elf/bin/xtensa-lx106-elf-gcc -c test.c -std=c11
In file included from test.c:2:
/home/mike/sandboxes/esp-quick-toolchain/xtensa-lx106-elf.x86_64/xtensa-lx106-elf/include/sys/pgmspace.h: In function 'pgm_read_byte_inlined':
/home/mike/sandboxes/esp-quick-toolchain/xtensa-lx106-elf.x86_64/xtensa-lx106-elf/include/sys/pgmspace.h:72:3: error: expected ')' before ':' token
   72 |   pgm_read_with_offset(addr, res);
      |   ^~~~~~~~~~~~~~~~~~~~

[earlephilhower]

Ah, got it. -std=c11 not --std=c++11. I can repo the failure now.

[earlephilhower]

Newlib is fixed and I'm rebuilding the whole kaboodle. Another hour or so and -gnu4 should be up.