| Message ID | 20150902075154.GB1047@jaguar.corp.atmel.com |
|---|---|
| State | New |
| Headers | show |
Committed. 2015-09-02 10:51 GMT+03:00 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com>: > Hi, > > This (rather trivial) patch fixes PR65210. The ICE happens because code > wasn't handling io_low attribute where it is supposed to. > > If this is ok, could someone commit please? I don't have commit > access. > > Regards > Senthil > > gcc/ChangeLog > > 2015-09-02 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > PR target/65210 > * config/avr/avr.c (avr_eval_addr_attrib): Look for io_low > attribute as well. > > gcc/testsuite/ChangeLog > > PR target/65210 > * gcc.target/avr/pr65210.c: New test. > > diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c > index bec9a8b..9f5bc88 100644 > --- gcc/config/avr/avr.c > +++ gcc/config/avr/avr.c > @@ -9069,6 +9069,8 @@ avr_eval_addr_attrib (rtx x) > if (SYMBOL_REF_FLAGS (x) & SYMBOL_FLAG_IO) > { > attr = lookup_attribute ("io", DECL_ATTRIBUTES (decl)); > + if (!attr || !TREE_VALUE (attr)) > + attr = lookup_attribute ("io_low", DECL_ATTRIBUTES (decl)); > gcc_assert (attr); > } > if (!attr || !TREE_VALUE (attr)) > diff --git gcc/testsuite/gcc.target/avr/pr65210.c gcc/testsuite/gcc.target/avr/pr65210.c > new file mode 100644 > index 0000000..1aed441 > --- /dev/null > +++ gcc/testsuite/gcc.target/avr/pr65210.c > @@ -0,0 +1,7 @@ > +/* { dg-do compile } */ > + > +/* This testcase exposes PR65210. Usage of the io_low attribute > + causes assertion failure because code only looks for the io > + attribute if SYMBOL_FLAG_IO is set. */ > + > +volatile char q __attribute__((io_low,address(0x81)));
Senthil Kumar Selvaraj schrieb: > This (rather trivial) patch fixes PR65210. The ICE happens because code > wasn't handling io_low attribute where it is supposed to. Hi Senthil, could you line out for what these new attributes are good for? The Compiler just maps the argument to a compile-time const, so the attributes do the same as a cast to a volatile address. Except that the user must know in advance what I/O Region is the right one. Supporting similar RELOCs for io* doesn't make hardly any sense, or are there Plans to add respective RELOCs (and ones for bitfields)? Thanks, Johann
On Tue, Sep 08, 2015 at 09:16:35AM +0200, Georg-Johann Lay wrote: > Senthil Kumar Selvaraj schrieb: > > This (rather trivial) patch fixes PR65210. The ICE happens because code > > wasn't handling io_low attribute where it is supposed to. > > Hi Senthil, could you line out for what these new attributes are good for? > The Compiler just maps the argument to a compile-time const, so the > attributes do the same as a cast to a volatile address. Except that the > user must know in advance what I/O Region is the right one. IIRC, Joern Rennecke added these so you could have extern declarations (without actual addresses) specifying that they will be in IO address space, so that the compiler can generate IN/OUT or the bit addressable instructions (SBI/CBI etc..). For example, extern char volatile SOMEPORT __attribute__((io_low)); int main() { SOMEPORT |= 1; } $ ~/avr/install/bin/avr-gcc -mmcu=avr5 useio.c -Os -S $ cat useio.s <snip> sbi SOMEPORT-32,0 ldi r24,0 ldi r25,0 ret </snip> whereas one without io_low will generate a lds, ori, sts sequence. > > Supporting similar RELOCs for io* doesn't make hardly any sense, or are > there Plans to add respective RELOCs (and ones for bitfields)? binutils already has those - R_AVR_PORT6 and R_AVR_PORT5 do that. This was added back in 2014 (https://sourceware.org/ml/binutils/2014-06/msg00122.html).
diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c index bec9a8b..9f5bc88 100644 --- gcc/config/avr/avr.c +++ gcc/config/avr/avr.c @@ -9069,6 +9069,8 @@ avr_eval_addr_attrib (rtx x) if (SYMBOL_REF_FLAGS (x) & SYMBOL_FLAG_IO) { attr = lookup_attribute ("io", DECL_ATTRIBUTES (decl)); + if (!attr || !TREE_VALUE (attr)) + attr = lookup_attribute ("io_low", DECL_ATTRIBUTES (decl)); gcc_assert (attr); } if (!attr || !TREE_VALUE (attr)) diff --git gcc/testsuite/gcc.target/avr/pr65210.c gcc/testsuite/gcc.target/avr/pr65210.c new file mode 100644 index 0000000..1aed441 --- /dev/null +++ gcc/testsuite/gcc.target/avr/pr65210.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ + +/* This testcase exposes PR65210. Usage of the io_low attribute + causes assertion failure because code only looks for the io + attribute if SYMBOL_FLAG_IO is set. */ + +volatile char q __attribute__((io_low,address(0x81)));