Hi David, that is an excellent idea. The attached patch implements that idea (slightly different) and almost makes clang happy. It know fails with:
> /usr/avr/include/avr/wdt.h:436:5: error: invalid operand for instruction > "in __tmp_reg__,__SREG__" "\n\t" > ^ > <inline asm>:1:5: note: instantiated into assembly here > in __tmp_reg__,__SREG__ > ^ So I guess for my application I know only have to teach clang the value of __tmp_reg__ (and maybe also a few other constants) to get it compiling :-) Kind regards, Marian On Fri, 15 Oct 2021 09:49:30 +0200 David Brown <[email protected]> wrote: > On 13/10/2021 18:05, Marian Buschsieweke wrote: > > Hi together, > > > > parts of the avrlibc headers are not compatible with clang. (The use case is > > not to compile avrlibc itself with clang, but rather an application using > > a vanilla GCC compiled avrlibc.) However, the issues are not always trivial > > to > > fix. > > > > E.g. in avr/wdt.h there are several instances of code like this: > > > >> static __inline__ > >> __attribute__ ((__always_inline__)) > >> void wdt_enable (const uint8_t value) > >> { > >> if (_SFR_IO_REG_P (_WD_CONTROL_REG)) > >> { > >> __asm__ __volatile__ ( > >> [...] > >> : /* no outputs */ > >> : "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)), > >> [...] > >> : "r0" > >> ); > >> } > >> else { > >> /* variant not using _WD_CONTROL_REG as immediate */ > >> [...] > >> > >> } > [...] > > It's been a /long/ time since I have done anything with the AVR, and I > don't have a modern avr gcc compiler (or any clang avr compiler at all) > handy for testing. However, I have an idea you can try: > > "I" (_SFR_IO_ADDR(_WD_CONTROL_REG) || 1) > > If "_SFR_IO_ADDR(_WD_CONTROL_REG)" evaluates to false, then this > expression would give "1", which clang might be happier with. >
diff --git a/trunk/avr-libc/include/avr/interrupt.h b/trunk/avr-libc/include/avr/interrupt.h
index a36e2ba6..844289df 100644
--- a/trunk/avr-libc/include/avr/interrupt.h
+++ b/trunk/avr-libc/include/avr/interrupt.h
@@ -125,9 +125,9 @@
# define ISR(vector, [attributes])
#else /* real code */
-#if (__GNUC__ == 4 && __GNUC_MINOR__ >= 1) || (__GNUC__ > 4)
+#if ((__GNUC__ == 4 && __GNUC_MINOR__ >= 1) || (__GNUC__ > 4)) && !defined(__clang__)
# define __INTR_ATTRS __used__, __externally_visible__
-#else /* GCC < 4.1 */
+#else /* GCC < 4.1 or clang */
# define __INTR_ATTRS __used__
#endif
diff --git a/trunk/avr-libc/include/avr/wdt.h b/trunk/avr-libc/include/avr/wdt.h
index 73517e3e..8d6253bf 100644
--- a/trunk/avr-libc/include/avr/wdt.h
+++ b/trunk/avr-libc/include/avr/wdt.h
@@ -41,6 +41,13 @@
#include <avr/io.h>
#include <stdint.h>
+/* Workaround for clang: This gives the I/O address of the given register if it can be used
+ * as immediate value, otherwise an incorrect address is returned that does fulfill the constraint
+ * "I" but must not be used. The programmer has to make sure that inline assembly using this macro
+ * is only executed, if _SFR_IO_REG_P(x) is true
+ */
+#define _SFR_IO_ADDR_WORKAROUND(x) (_SFR_IO_ADDR(x) & 0x3f)
+
/** \file */
/** \defgroup avr_watchdog <avr/wdt.h>: Watchdog timer handling
\code #include <avr/wdt.h> \endcode
@@ -266,7 +273,7 @@ void wdt_enable (const uint8_t value)
: /* no outputs */
: [CCPADDRESS] "n" (_SFR_MEM_ADDR(CCP)),
[SIGNATURE] "r" ((uint8_t)0xD8),
- [WDTREG] "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)),
+ [WDTREG] "I" (_SFR_IO_ADDR_WORKAROUND(_WD_CONTROL_REG)),
[WDVALUE] "r" ((uint8_t)((value & 0x08 ? _WD_PS3_MASK : 0x00)
| _BV(WDE) | (value & 0x07) ))
: "r0"
@@ -282,7 +289,7 @@ void wdt_enable (const uint8_t value)
"sts %[WDTREG],%[WDVALUE]" "\n\t"
"out __SREG__,__tmp_reg__" "\n\t"
: /* no outputs */
- : [CCPADDRESS] "I" (_SFR_IO_ADDR(CCP)),
+ : [CCPADDRESS] "I" (_SFR_IO_ADDR_WORKAROUND(CCP)),
[SIGNATURE] "r" ((uint8_t)0xD8),
[WDTREG] "n" (_SFR_MEM_ADDR(_WD_CONTROL_REG)),
[WDVALUE] "r" ((uint8_t)((value & 0x08 ? _WD_PS3_MASK : 0x00)
@@ -300,9 +307,9 @@ void wdt_enable (const uint8_t value)
"out %[WDTREG],%[WDVALUE]" "\n\t"
"out __SREG__,__tmp_reg__" "\n\t"
: /* no outputs */
- : [CCPADDRESS] "I" (_SFR_IO_ADDR(CCP)),
+ : [CCPADDRESS] "I" (_SFR_IO_ADDR_WORKAROUND(CCP)),
[SIGNATURE] "r" ((uint8_t)0xD8),
- [WDTREG] "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)),
+ [WDTREG] "I" (_SFR_IO_ADDR_WORKAROUND(_WD_CONTROL_REG)),
[WDVALUE] "r" ((uint8_t)((value & 0x08 ? _WD_PS3_MASK : 0x00)
| _BV(WDE) | (value & 0x07) ))
: "r0"
@@ -350,7 +357,7 @@ void wdt_disable (void)
: /*no output */
: [CCPADDRESS] "n" (_SFR_MEM_ADDR(CCP)),
[SIGNATURE] "r" ((uint8_t)0xD8),
- [WDTREG] "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)),
+ [WDTREG] "I" (_SFR_IO_ADDR_WORKAROUND(_WD_CONTROL_REG)),
[TEMP_WD] "d" (temp_wd),
[WDVALUE] "n" (1 << WDE)
: "r0"
@@ -369,7 +376,7 @@ void wdt_disable (void)
"sts %[WDTREG],%[TEMP_WD]" "\n\t"
"out __SREG__,__tmp_reg__" "\n\t"
: /*no output */
- : [CCPADDRESS] "I" (_SFR_IO_ADDR(CCP)),
+ : [CCPADDRESS] "I" (_SFR_IO_ADDR_WORKAROUND(CCP)),
[SIGNATURE] "r" ((uint8_t)0xD8),
[WDTREG] "n" (_SFR_MEM_ADDR(_WD_CONTROL_REG)),
[TEMP_WD] "d" (temp_wd),
@@ -390,9 +397,9 @@ void wdt_disable (void)
"out %[WDTREG],%[TEMP_WD]" "\n\t"
"out __SREG__,__tmp_reg__" "\n\t"
: /*no output */
- : [CCPADDRESS] "I" (_SFR_IO_ADDR(CCP)),
+ : [CCPADDRESS] "I" (_SFR_IO_ADDR_WORKAROUND(CCP)),
[SIGNATURE] "r" ((uint8_t)0xD8),
- [WDTREG] "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)),
+ [WDTREG] "I" (_SFR_IO_ADDR_WORKAROUND(_WD_CONTROL_REG)),
[TEMP_WD] "d" (temp_wd),
[WDVALUE] "n" (1 << WDE)
: "r0"
@@ -416,7 +423,7 @@ void wdt_enable (const uint8_t value)
"out __SREG__,__tmp_reg__" "\n\t"
"out %0, %2" "\n \t"
: /* no outputs */
- : "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)),
+ : "I" (_SFR_IO_ADDR_WORKAROUND(_WD_CONTROL_REG)),
"r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))),
"r" ((uint8_t) ((value & 0x08 ? _WD_PS3_MASK : 0x00) |
_BV(WDE) | (value & 0x07)) )
@@ -459,7 +466,7 @@ void wdt_disable (void)
"out %[WDTREG],__zero_reg__" "\n\t"
"out __SREG__,__tmp_reg__" "\n\t"
: [TEMPREG] "=d" (temp_reg)
- : [WDTREG] "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)),
+ : [WDTREG] "I" (_SFR_IO_ADDR_WORKAROUND(_WD_CONTROL_REG)),
[WDCE_WDE] "n" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE)))
: "r0"
);
pgpfDkR1_UQOS.pgp
Description: OpenPGP digital signature
