Arduino IDE compile error with new ESP8266 core 3.0.0

[v-a-d-e-r]

/arduino/libraries/Brzo_I2C/brzo_i2c.c: In function 'brzo_i2c_write':
/arduino/libraries/Brzo_I2C/brzo_i2c.c:72:2: error: cannot find a register in class 'RL_REGS' while reloading 'asm'
72 | asm volatile (
| ^~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:72:2: error: 'asm' operand has impossible constraints
exit status 1
Fehler beim Kompilieren für das Board Generic ESP8266 Module.

Can this be easy fixed? Thanks.
I'm using latest brzo version...

[JimDrewGH]

I can confirm the same for anything using brzo_i2c with v3.0.0. I have used all previous versions of the ESP8266 core (2.7.4 through 2.3.0) and they all work fine with v1.3.3 of the Brzo library. The new v3.0.0 core generates the same error as shown above.

[pasko-zh]

Probably related to #40.

[pasko-zh]

Well, I am not an expert on GCC... I don't know what they exactly changed in the tool chain, like compiler switches or so. I am using many registers, so the compiler basically says "no more registers". But why suddenly they run out of registers, I really don't know.

Maybe better to ask at arduino core

[d-a-v]

I am not an real assembler guy but I can read with datasheet next to code.
Here's a patch that could be tested. You need to know that I did not test it, so I don't know if it still works and neither the impact on performances.
I reduced the requested register size (where I could understand that it is safe but @pasko-zh may advise) and with these changes gcc-10.2-xtensa is fine.

edit: Maybe this is a non-sense because one cannot use a non-32bit variable to be mapped to a register. gcc not complaining made me confident.

diff --git a/examples/ADT7420/ADT7420.ino b/examples/ADT7420/ADT7420.ino
index 66df3b4..64959e8 100644
--- a/examples/ADT7420/ADT7420.ino
+++ b/examples/ADT7420/ADT7420.ino
@@ -24,7 +24,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 // Analog Devices ADT7420 Datasheet
 // http://www.analog.com/en/products/analog-to-digital-converters/integrated-special-purpose-converters/integrated-temperature-sensors/adt7420.html
 
-#include "brzo_i2c\brzo_i2c.h"
+#include <brzo_i2c.h>
 
 uint8_t SDA_PIN = 5;
 uint8_t SCL_PIN = 4;
diff --git a/src/brzo_i2c.c b/src/brzo_i2c.c
index c797c25..5ac2973 100644
--- a/src/brzo_i2c.c
+++ b/src/brzo_i2c.c
@@ -66,7 +66,9 @@ void ICACHE_RAM_ATTR brzo_i2c_write(uint8_t *data, uint32_t no_of_bytes, bool re
 	if (i2c_error > 0) return;
 	uint8_t byte_to_send = i2c_slave_address << 1;
 	// Assembler Variables
-	uint32_t a_set = 0, a_repeated = 0, a_in_value = 0, a_temp1 = 0, a_bit_index = 0;
+	uint8_t a_repeated, a_bit_index = 0;
+	uint16_t a_in_value = 0, a_temp1 = 0;
+	uint32_t a_set = 0;
 	if (repeated_start == true) a_repeated = 1;
 	else a_repeated = 0;
 	asm volatile (
@@ -384,8 +386,9 @@ void ICACHE_RAM_ATTR brzo_i2c_read(uint8_t *data, uint32_t nr_of_bytes, bool rep
 	// Do not perform an i2c read if a previous i2c command has already failed
 	if (i2c_error > 0) return;
 	// Assembler Variables
-	uint32_t a_set = 0, a_repeated = 0, a_in_value = 0, a_temp1 = 0, a_temp2 = 0, a_bit_index = 0;
-	a_temp2 = 0;
+    uint8_t a_repeated, a_bit_index = 0;
+    uint16_t a_in_value = 0, a_temp1 = 0;
+	uint32_t a_set = 0, a_temp2;
 	if (repeated_start == true) a_repeated = 1;
 	else a_repeated = 0;
 	// a_temp2 holds 7 Bit slave address, with the LSB = 1 for i2c read

[v-a-d-e-r]

@d-a-v
Thanks for looking for this issue. I changed the code and get now this (IDE 1.8.13):
/arduino/libraries/Brzo_I2C/brzo_i2c.c: In function 'brzo_i2c_write':
/arduino/libraries/Brzo_I2C/brzo_i2c.c:69:3: error: expected expression before 'uint8_t'
69 | + uint8_t a_repeated, a_bit_index = 0;
| ^~~~~~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:70:3: error: expected expression before 'uint16_t'
70 | + uint16_t a_in_value = 0, a_temp1 = 0;
| ^~~~~~~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:71:3: error: expected expression before 'uint32_t'
71 | + uint32_t a_set = 0;
| ^~~~~~~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:72:30: error: 'a_repeated' undeclared (first use in this function)
72 | if (repeated_start == true) a_repeated = 1;
| ^~~~~~~~~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:72:30: note: each undeclared identifier is reported only once for each function it appears in
/arduino/libraries/Brzo_I2C/brzo_i2c.c:361:19: error: 'a_set' undeclared (first use in this function); did you mean 'fd_set'?
361 | : [r_set] "+r" (a_set), [r_repeated] "+r" (a_repeated), [r_temp1] "+r" (a_temp1), [r_in_value] "+r" (a_in_value), [r_error] "+r" (i2c_error), [r_bit_index] "+r" (a_bit_index), [r_adr_array_element] "+r" (&data[0]), [r_byte_to_send] "+r" (byte_to_send), [r_no_of_bytes] "+r" (no_of_bytes)
| ^~~~~
| fd_set
/arduino/libraries/Brzo_I2C/brzo_i2c.c:361:75: error: 'a_temp1' undeclared (first use in this function)
361 | : [r_set] "+r" (a_set), [r_repeated] "+r" (a_repeated), [r_temp1] "+r" (a_temp1), [r_in_value] "+r" (a_in_value), [r_error] "+r" (i2c_error), [r_bit_index] "+r" (a_bit_index), [r_adr_array_element] "+r" (&data[0]), [r_byte_to_send] "+r" (byte_to_send), [r_no_of_bytes] "+r" (no_of_bytes)
| ^~~~~~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:361:104: error: 'a_in_value' undeclared (first use in this function)
361 | : [r_set] "+r" (a_set), [r_repeated] "+r" (a_repeated), [r_temp1] "+r" (a_temp1), [r_in_value] "+r" (a_in_value), [r_error] "+r" (i2c_error), [r_bit_index] "+r" (a_bit_index), [r_adr_array_element] "+r" (&data[0]), [r_byte_to_send] "+r" (byte_to_send), [r_no_of_bytes] "+r" (no_of_bytes)
| ^~~~~~~~~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:361:165: error: 'a_bit_index' undeclared (first use in this function)
361 | : [r_set] "+r" (a_set), [r_repeated] "+r" (a_repeated), [r_temp1] "+r" (a_temp1), [r_in_value] "+r" (a_in_value), [r_error] "+r" (i2c_error), [r_bit_index] "+r" (a_bit_index), [r_adr_array_element] "+r" (&data[0]), [r_byte_to_send] "+r" (byte_to_send), [r_no_of_bytes] "+r" (no_of_bytes)
| ^~~~~~~~~~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c: In function 'brzo_i2c_read':
/arduino/libraries/Brzo_I2C/brzo_i2c.c:390:3: error: expected expression before 'uint32_t'
390 | + uint32_t a_set = 0, a_temp2;
| ^~~~~~~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:391:2: error: 'a_temp2' undeclared (first use in this function); did you mean 'a_temp1'?
391 | a_temp2 = 0;
| ^~~~~~~
| a_temp1
/arduino/libraries/Brzo_I2C/brzo_i2c.c:748:19: error: 'a_set' undeclared (first use in this function); did you mean 'fd_set'?
748 | : [r_set] "+r" (a_set), [r_repeated] "+r" (a_repeated), [r_temp1] "+r" (a_temp1), [r_in_value] "+r" (a_in_value), [r_error] "+r" (i2c_error), [r_bit_index] "+r" (a_bit_index), [r_adr_array_element] "+r" (&data[0]), [r_temp2] "+r" (a_temp2), [r_nr_of_bytes] "+r" (nr_of_bytes)
| ^~~~~
| fd_set
/arduino/libraries/Brzo_I2C/brzo_i2c.c: In function 'brzo_i2c_write':
/arduino/libraries/Brzo_I2C/brzo_i2c.c:73:2: error: invalid lvalue in 'asm' output 0
73 | asm volatile (
| ^~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:73:2: error: invalid lvalue in 'asm' output 1
/arduino/libraries/Brzo_I2C/brzo_i2c.c:73:2: error: invalid lvalue in 'asm' output 2
/arduino/libraries/Brzo_I2C/brzo_i2c.c:73:2: error: invalid lvalue in 'asm' output 3
/arduino/libraries/Brzo_I2C/brzo_i2c.c:73:2: error: invalid lvalue in 'asm' output 5
/arduino/libraries/Brzo_I2C/brzo_i2c.c: In function 'brzo_i2c_read':
/arduino/libraries/Brzo_I2C/brzo_i2c.c:397:2: error: invalid lvalue in 'asm' output 0
397 | asm volatile (
| ^~~
/arduino/libraries/Brzo_I2C/brzo_i2c.c:397:2: error: invalid lvalue in 'asm' output 7
exit status 1

[pasko-zh]

@d-a-v : Thanks for your support. In general, I do not mind changing the code ;-) However, I do not yet understand what exactly is the issue here, i.e., why is the compilation no longer possible.
Before changing the code I (we) should know what exactly causes the failure. It is (most likely) related to the change in GCC version, or some config files, or compiler switches, or ...

@ALL: I need some help here, because I don't have deep knowledge about the tool chain

[v-a-d-e-r]

BTW: Isn't this line (71) nonsense?
// Assembler Variables
uint32_t a_set = 0, a_repeated = 0, a_in_value = 0, a_temp1 = 0, a_bit_index = 0;
if (repeated_start == true) a_repeated = 1;
else a_repeated = 0; // <<=== It's already defined 2 lines above as 0!
asm volatile (

[d-a-v]

@v-a-d-e-r This is not really a nonsense. It is a common practice to initialize a variable when declaring it. Also, modern compilers may optimize this and initialize only once in the resulting assembly code if it can be detected that the variable is not used between the two initializations.

@v-a-d-e-r What I posted is a diff file. You need to remove lines with - and replace them with lines with + (without + in new lines).

@pasko-zh The 3.0.0 esp8266 arduino core release uses gcc-xtensa v10.2. The previous releases were using gcc-xtensa 4.8. Code is compiled with -Os. Optimization features of gcc are better (we can hope for sure) and probably make a better use of registers. Your inline assembly code requires lots of registers and gcc cannot makes its own register optimization because your code reserves too many 32bits registers. What I tried is to tell gcc to use smaller registers (without knowing whether it would work - I wanted at least to check if the error message would disappear).
In the meantime @mcspr from #40 made a successful try. This still has to be confirmed by you @pasko-zh, especially by checking if there's a performance penalty.

[v-a-d-e-r]

@d-a-v ok, thanks for explaining this. It looked a bit curious to me on the first view... ;-) And yes, I did exchange the code, not just adding yours.

[d-a-v]

@v-a-d-e-r I was suggesting to remove the + at the beginning of each because of the errors you reported above, one of which I reproduce here:

/arduino/libraries/Brzo_I2C/brzo_i2c.c: In function 'brzo_i2c_write':
/arduino/libraries/Brzo_I2C/brzo_i2c.c:69:3: error: expected expression before 'uint8_t'
69 | + uint8_t a_repeated, a_bit_index = 0;
   | ^~~~~~~

[v-a-d-e-r]

Hmm, that what I posted above was just a "copy and paste" from the compiler output... :-/ The '+' is NOT in the source code of course

[d-a-v]

Sorry I might have misunderstood

[pasko-zh]

@d-a-v : Thanks, I will have some thoughts how and where I could reduce the amount of registers.

Maybe some words why I did it this way: At the time when I decided to write brzo_i2c, I wanted to push our little esp8266 to the limits, in the sense of clock speed and precision of i2c signals. And I thus wanted to reduce any effects that might lower this, for instance push register values to stack and pop them back again, or allow interrupts, etc.
Concerning register usage, it was on the edge from the beginning. So maybe, I went a bit too far...

[d-a-v]

It is amazing how people like to push this particular beast further the limits :)

[sh-user]

I compared the compilation speed in Arduino IDE 1.8.15 ESP8266
3.0.0 (gcc-xtensa v10.2) - first compilation 10 minutes, second compilation 10 minutes
2.7.4 (gcc-xtensa v4.8) - first compilation 5 minutes, second compilation less than 1 minute

[pasko-zh]

@sh-user Well, the question is what do we get for increased compilation time? Smaller code? Faster code? Do you have some measurements on that?

[v-a-d-e-r]

Yep, the longer compile time was another reason for reverting to core 2.7.4 for now.... :-/

[sh-user]

@pasko-zh 4 KB more free heap

[d-a-v]

and 16KB more with a new option in the Arduino IDE menu

[sh-user]

@d-a-v how to get 16KB? what option in the Arduino IDE menu?

[d-a-v]

Tools>MMU>3rd option 2nd heap (shared) and documentation.

[JimDrewGH]

v3.0.1 of the EPS8266 core was just released, and of course the same issue with compiling exists. Do you think there will be a fix for this in the near future?

[wladimir-computin]

Hey guys,

I might have found a fix for this bug: Please take a look at my commit

I'm basically just using uint16_t registers and only one uint32_t, because in my understanding of the asm code (which may be absolutely wrong) these remaining registers never store more than 16bit and L16UI returns 16bit.
It compiles (Arduino ESP8266 3.0.2) and was I've tested it successfully with a SH1106 OLED. YMMV.

Disclaimer: I have only rudimentary asm knowledge, please don't hit me :D

EDIT: The only downside I found is, that iteration_scl_clock_stretch and clock_stretch_time_out_usec must be changed to uint16_t and caped at 65535, because of the reduced register size of temp_1. Still fine for me, since I never changed iteration_scl_clock_stretch in my app and it's 100 by default. But it might be an issue for others.

[d-a-v]

@wladimir-computin Since you are successfully running your patch, why don't you turn it into a pull-request ?

[pasko-zh]

Hi there!
Thanks for your support.
I was away for a couple of weeks... and now back in a new job. As soon as I find time, I will have look at it and do some tests.

[kraygy]

Hi, Any updates on this? thanks.

[ifeghali]

Hey guys,

I might have found a fix for this bug: Please take a look at my commit

I'm basically just using uint16_t registers and only one uint32_t, because in my understanding of the asm code (which may be absolutely wrong) these remaining registers never store more than 16bit and L16UI returns 16bit. It compiles (Arduino ESP8266 3.0.2) and was I've tested it successfully with a SH1106 OLED. YMMV.

Disclaimer: I have only rudimentary asm knowledge, please don't hit me :D

EDIT: The only downside I found is, that iteration_scl_clock_stretch and clock_stretch_time_out_usec must be changed to uint16_t and caped at 65535, because of the reduced register size of temp_1. Still fine for me, since I never changed iteration_scl_clock_stretch in my app and it's 100 by default. But it might be an issue for others.

works for me on SSD1306 and core 3.0.2

[Thelmos]

Do we have some feedback on @wladimir-computin fix? (besides @ifeghali test)
Can we use his patch without any side effect?

[ifeghali]

Do we have some feedback on @wladimir-computin fix? (besides @ifeghali test) Can we use his patch without any side effect?

It did compile successfully but I had to drop BRZO for some problem I don't remember exactly. My project was finished and board was sent to customer so I can't test it again but I have more boards on my way from China. Will give it another try and let you know. It's going to take a few weeks though.

[fusionstream]

Will the PR be accepted?