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 <da...@westcontrol.com> 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
