search

PaulStoffregen / Wire

Wire library used on Teensy boards
https://www.pjrc.com/teensy/td_libs_Wire.html
21 stars 50 forks source link

GCC 6 compile errors #9

Open FrankBoesing opened 6 years ago

FrankBoesing commented 6 years ago

Using ARM GCC 6 / Q2 , i get the following errors: ` "C:\Arduino\hardware\teensy/../tools/arm6/bin/arm-none-eabi-g++" -c -O2 -g -Wall -ffunction-sections -fdata-sections -nostdlib -MMD -fno-exceptions -felide-constructors -std=gnu++14 -fno-rtti -mthumb -mcpu=cortex-m4 -mfloat-abi=hard -mfpu=fpv4-sp-d16 -fsingle-precision-constant -DMK66FX1M0 -DTEENSYDUINO=140 -DARDUINO=10804 -DF_CPU=240000000 -DUSB_RAWHID -DLAYOUT_US_ENGLISH "@f:\arduino_build_3190/sketch/defs.h" "-IC:\Arduino\hardware\teensy\avr\cores\teensy3" "-IC:\Arduino\hardware\teensy\avr\libraries\Audio" "-IC:\Arduino\hardware\teensy\avr\libraries\SPI" "-IC:\Arduino\hardware\teensy\avr\libraries\SD" "-IC:\Arduino\hardware\teensy\avr\libraries\SerialFlash" "-IC:\Arduino\hardware\teensy\avr\libraries\Wire" "-IC:\Arduino\hardware\teensy\avr\libraries\Wire\utility" "C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp" -o "f:\arduino_build_3190\libraries\Wire\WireKinetis.cpp.o" C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:803:54: error: reinterpret_cast from integer to pointer

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:800:50:

define MAKE_CONST(x) (__builtin_constant_p(x) ? (x) : (x))

                                              ~~~

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:803:54:

constexpr uintptr_t i2c0_addr = uintptr_t(MAKE_CONST(&KINETIS_I2C0));

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:800:51: note: in definition of macro 'MAKE_CONST'

define MAKE_CONST(x) (__builtin_constant_p(x) ? (x) : (x))

                                               ^

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:808:54: error: reinterpret_cast from integer to pointer

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:800:50:

define MAKE_CONST(x) (__builtin_constant_p(x) ? (x) : (x))

                                              ~~~

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:808:54:

constexpr uintptr_t i2c1_addr = uintptr_t(MAKE_CONST(&KINETIS_I2C1));

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:800:51: note: in definition of macro 'MAKE_CONST'

define MAKE_CONST(x) (__builtin_constant_p(x) ? (x) : (x))

                                               ^

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:813:54: error: reinterpret_cast from integer to pointer

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:800:50:

define MAKE_CONST(x) (__builtin_constant_p(x) ? (x) : (x))

                                              ~~~

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:813:54:

constexpr uintptr_t i2c2_addr = uintptr_t(MAKE_CONST(&KINETIS_I2C2));

C:\Arduino\hardware\teensy\avr\libraries\Wire\WireKinetis.cpp:800:51: note: in definition of macro 'MAKE_CONST'

define MAKE_CONST(x) (__builtin_constant_p(x) ? (x) : (x))

                                              ^

`

FrankBoesing commented 6 years ago

I think constexpr uintptr_t i2c0_addr = uintptr_t(MAKE_CONST(&KINETIS_I2C1.A1)); would work (no compile errors on Gcc5 + GCC6) - but I could not test with I2C hardware.

FlorianFranzen commented 6 years ago

Might be this change in gcc 4.9.2.

PaulStoffregen commented 6 years ago

Unlikely. It works without any errors on gcc 5.4.

FlorianFranzen commented 6 years ago

@PaulStoffregen: Why is this macro used in the first place? It seems to return ((x)), no matter if respected x can be folded or not.

Trass3r commented 6 years ago

Looks like that old hack to circumvent the constexpr reinterpret_cast limitation does not work anymore in gcc and clang.

FlorianFranzen commented 6 years ago

As a workaround I added KINETIS_I2CX_ADDR as a macros that only contains the address to kinetis.h and used them inside KINETIS_I2CX and without the MAKE_CONST(...) inside the Wire library:

--- core/kinetis.h  2018-06-20 20:47:18.000000000 +0200
+++ core/kinetis.h  2018-07-02 18:57:59.911936300 +0200
@@ -4566,10 +4566,14 @@
    volatile uint8_t    SLTH;
    volatile uint8_t    SLTL;
 } KINETIS_I2C_t;
-#define KINETIS_I2C0       (*(KINETIS_I2C_t *)0x40066000)
-#define KINETIS_I2C1       (*(KINETIS_I2C_t *)0x40067000)
-#define KINETIS_I2C2       (*(KINETIS_I2C_t *)0x400E6000)
-#define KINETIS_I2C3       (*(KINETIS_I2C_t *)0x400E7000)
+#define KINETIS_I2C0_ADDR  0x40066000
+#define KINETIS_I2C1_ADDR  0x40067000
+#define KINETIS_I2C2_ADDR  0x400E6000
+#define KINETIS_I2C3_ADDR  0x400E7000
+#define KINETIS_I2C0       (*(KINETIS_I2C_t *) KINETIS_I2C0_ADDR)
+#define KINETIS_I2C1       (*(KINETIS_I2C_t *) KINETIS_I2C1_ADDR)
+#define KINETIS_I2C2       (*(KINETIS_I2C_t *) KINETIS_I2C2_ADDR)
+#define KINETIS_I2C3       (*(KINETIS_I2C_t *) KINETIS_I2C3_ADDR)
 #define I2C0_A1            (KINETIS_I2C0.A1)       // I2C Address Register 1
 #define I2C0_F         (KINETIS_I2C0.F)        // I2C Frequency Divider register
 #define I2C_F_DIV20            ((uint8_t)0x00)

--- src/WireKinetis.cpp 2018-07-02 18:51:19.323864755 +0200
+++ src/WireKinetis.cpp 2018-07-02 18:50:23.742853593 +0200
@@ -874,31 +871,23 @@
 };
 #endif

-// Helper to transform a non-constant expression of the form
-// &(*(KINETIS_I2C_t *)0x40066000)
-// into a compile time constant.
-#define MAKE_CONST(x) (__builtin_constant_p(x) ? (x) : (x))
-
 #ifdef WIRE_IMPLEMENT_WIRE
-constexpr uintptr_t i2c0_addr = uintptr_t(MAKE_CONST(&KINETIS_I2C0));
+constexpr uintptr_t i2c0_addr = uintptr_t(KINETIS_I2C0_ADDR);
 TwoWire Wire(i2c0_addr, TwoWire::i2c0_hardware);
 void i2c0_isr(void) { Wire.isr(); }
 #endif
 #ifdef WIRE_IMPLEMENT_WIRE1
-constexpr uintptr_t i2c1_addr = uintptr_t(MAKE_CONST(&KINETIS_I2C1));
+constexpr uintptr_t i2c1_addr = uintptr_t(KINETIS_I2C1_ADDR);
 TwoWire Wire1(i2c1_addr, TwoWire::i2c1_hardware);
 void i2c1_isr(void) { Wire1.isr(); }
 #endif
 #ifdef WIRE_IMPLEMENT_WIRE2
-constexpr uintptr_t i2c2_addr = uintptr_t(MAKE_CONST(&KINETIS_I2C2));
+constexpr uintptr_t i2c2_addr = uintptr_t(KINETIS_I2C2_ADDR);
 TwoWire Wire2(i2c2_addr, TwoWire::i2c2_hardware);
 void i2c2_isr(void) { Wire2.isr(); }
 #endif
 #ifdef WIRE_IMPLEMENT_WIRE3
-constexpr uintptr_t i2c3_addr = uintptr_t(MAKE_CONST(&KINETIS_I2C3));
+constexpr uintptr_t i2c3_addr = uintptr_t(KINETIS_I2C3_ADDR);
 TwoWire Wire3(i2c3_addr, TwoWire::i2c3_hardware);
 void i2c3_isr(void) { Wire3.isr(); }
 #endif

You could probably drop the surrounding uintptr_t(...) as well.

What was the reason to use this construct in the first place? For something as trivial as this, a hack like that just makes the code harder to read and maintain.