Re: [patch,avr,applied] PR target/112952 Fix attribute "io" et al. handling.
Am 12.01.24 um 04:37 schrieb Jan-Benedict Glaw: On Thu, 2024-01-04 17:28:02 +0100, Georg-Johann Lay wrote: This fixes the avr-specific attributes io, io_low and address, that are all basically the same except that io and io_low imply assertions on allowed addressing modes. --- a/gcc/config/avr/avr.cc +++ b/gcc/config/avr/avr.cc [...] @@ -10385,12 +10389,10 @@ avr_handle_addr_attribute (tree *node, tree name, tree args, } else if (io_p && (!tree_fits_shwi_p (arg) - || !(strcmp (IDENTIFIER_POINTER (name), "io_low") == 0 - ? low_io_address_operand : io_address_operand) -(GEN_INT (TREE_INT_CST_LOW (arg)), QImode))) + || ! IN_RANGE (TREE_INT_CST_LOW (arg), io_start, io_end))) { - warning_at (loc, OPT_Wattributes, "%qE attribute address " - "out of range", name); + warning_at (loc, OPT_Wattributes, "%qE attribute address out of " + "range 0x%x...0x%x", name, (int) io_start, (int) io_end); *no_add = true; } else Building with a recent GCC, this results in a new warning (here forced to an error with --enable-werror-alway--enable-werror-always): /var/lib/laminar/run/gcc-avr-elf/64/local-toolchain-install/bin/g++ -fno-PIE -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace -o avr.o -MT avr.o -MMD -MP -MF ./.deps/avr.TPo ../../gcc/gcc/config/avr/avr.cc ../../gcc/gcc/config/avr/avr.cc: In function 'tree_node* avr_handle_addr_attribute(tree_node**, tree, tree, int, bool*)': ../../gcc/gcc/config/avr/avr.cc:10391:45: error: unquoted sequence of 3 consecutive punctuation characters '...' in format [-Werror=format-diag] 10391 | warning_at (loc, OPT_Wattributes, "%qE attribute address out of " | ^~~ 10392 | "range 0x%x...0x%x", name, (int) io_start, (int) io_end); | ~~~ cc1plus: all warnings being treated as errors make[1]: *** [Makefile:2554: avr.o] Error 1 make[1]: Leaving directory '/var/lib/laminar/run/gcc-avr-elf/64/toolchain-build/gcc' make: *** [Makefile:4676: all-gcc] Error 2 I think this should be "%<...%>". MfG, JBG Hi, thanks for sorting this out. I would install the patch below. I must admit that I don't understand that warning and what is illegal about having ellipses in a format string at that place. That warning isn't even documented, at least as of 2024-02-12 https://gcc.gnu.org/onlinedocs/gcc/Option-Index.html There should be no quotes around the ellipses, they are intended as real ellipses. Plus, I saw in other modules that it warns about format strings like printf (";%i", 10); Why is ";" not allowed there? Johann -- diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc index 0cdd035fa1a..4bc3cf929de 100644 --- a/gcc/config/avr/avr.cc +++ b/gcc/config/avr/avr.cc @@ -10388,8 +10388,8 @@ avr_handle_addr_attribute (tree *node, tree name, tree args, && (!tree_fits_shwi_p (arg) || ! IN_RANGE (TREE_INT_CST_LOW (arg), io_start, io_end))) { - warning_at (loc, OPT_Wattributes, "%qE attribute address out of " - "range 0x%x...0x%x", name, (int) io_start, (int) io_end); + warning_at (loc, OPT_Wattributes, "%qE attribute address out of range" + " 0x%x%s0x%x", name, (int) io_start, "...", (int) io_end); *no_add = true; } else
Re: [patch,avr,applied] PR target/112952 Fix attribute "io" et al. handling.
On Thu, 2024-01-04 17:28:02 +0100, Georg-Johann Lay wrote: > This fixes the avr-specific attributes io, io_low and address, > that are all basically the same except that io and io_low imply > assertions on allowed addressing modes. > --- a/gcc/config/avr/avr.cc > +++ b/gcc/config/avr/avr.cc [...] > @@ -10385,12 +10389,10 @@ avr_handle_addr_attribute (tree *node, tree name, > tree args, > } >else if (io_p > && (!tree_fits_shwi_p (arg) > -|| !(strcmp (IDENTIFIER_POINTER (name), "io_low") == 0 > - ? low_io_address_operand : io_address_operand) > - (GEN_INT (TREE_INT_CST_LOW (arg)), QImode))) > +|| ! IN_RANGE (TREE_INT_CST_LOW (arg), io_start, io_end))) > { > - warning_at (loc, OPT_Wattributes, "%qE attribute address " > - "out of range", name); > + warning_at (loc, OPT_Wattributes, "%qE attribute address out of " > + "range 0x%x...0x%x", name, (int) io_start, (int) io_end); > *no_add = true; > } >else Building with a recent GCC, this results in a new warning (here forced to an error with --enable-werror-alway--enable-werror-always): /var/lib/laminar/run/gcc-avr-elf/64/local-toolchain-install/bin/g++ -fno-PIE -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common -DHAVE_CONFIG_H -fno-PIE -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace -o avr.o -MT avr.o -MMD -MP -MF ./.deps/avr.TPo ../../gcc/gcc/config/avr/avr.cc ../../gcc/gcc/config/avr/avr.cc: In function 'tree_node* avr_handle_addr_attribute(tree_node**, tree, tree, int, bool*)': ../../gcc/gcc/config/avr/avr.cc:10391:45: error: unquoted sequence of 3 consecutive punctuation characters '...' in format [-Werror=format-diag] 10391 | warning_at (loc, OPT_Wattributes, "%qE attribute address out of " | ^~~ 10392 | "range 0x%x...0x%x", name, (int) io_start, (int) io_end); | ~~~ cc1plus: all warnings being treated as errors make[1]: *** [Makefile:2554: avr.o] Error 1 make[1]: Leaving directory '/var/lib/laminar/run/gcc-avr-elf/64/toolchain-build/gcc' make: *** [Makefile:4676: all-gcc] Error 2 I think this should be "%<...%>". MfG, JBG -- signature.asc Description: PGP signature
[patch,avr,applied] PR target/112952 Fix attribute "io" et al. handling.
This fixes the avr-specific attributes io, io_low and address, that are all basically the same except that io and io_low imply assertions on allowed addressing modes. It also improves some diagnostics. Johann -- gcc/ PR target/112952 * config/avr/avr.cc (avr_handle_addr_attribute): Also print valid range when diagnosing attribute "io" and "io_low" are out of range. (avr_eval_addr_attrib): Don't ICE on empty address at that place. (avr_insert_attributes): Reject if attribute "address", "io" or "io_low" in contexts other than static storage. (avr_asm_output_aligned_decl_common): Move output of decls with attribute "address", "io", and "io_low" to... (avr_output_addr_attrib): ...this new function. (avr_asm_asm_output_aligned_bss): Remove output for decls with attribute "address", "io", and "io_low". (avr_encode_section_info): Rectify handling of decls with attribute "address", "io", and "io_low". gcc/testsuite/ PR target/112952 * gcc.target/avr/attribute-io.h: New file. * gcc.target/avr/pr112952-0.c: New test. * gcc.target/avr/pr112952-1.c: New test. * gcc.target/avr/pr112952-2.c: New test. * gcc.target/avr/pr112952-3.c: New test.diff --git a/gcc/testsuite/gcc.target/avr/attribute-io.h b/gcc/testsuite/gcc.target/avr/attribute-io.h new file mode 100644 index 000..39abd4ef59f --- /dev/null +++ b/gcc/testsuite/gcc.target/avr/attribute-io.h @@ -0,0 +1,74 @@ +/* { dg-do run } */ +/* { dg-options "-Os -save-temps" } */ + +__attribute__((address(1234))) +int g_1234; + +__attribute__((weak, address(4321))) +int w_4321; + +__attribute__((address(5678))) +static int l_5678; + +__attribute__((io_low(__AVR_SFR_OFFSET__ + 3))) +volatile unsigned char g_low; + +__attribute__((weak, io_low(__AVR_SFR_OFFSET__ + 2))) +volatile unsigned char w_low; + +__attribute__((io_low(__AVR_SFR_OFFSET__ + 1))) +static volatile unsigned char l_low; + +__attribute__((io(__AVR_SFR_OFFSET__ + 35))) +volatile unsigned char g_io; + +__attribute__((weak, io(__AVR_SFR_OFFSET__ + 34))) +volatile unsigned char w_io; + +__attribute__((io(__AVR_SFR_OFFSET__ + 33))) +static volatile unsigned char l_io; + +#define CMP(SYM, VAL)\ + do { \ +unsigned x; \ +__asm ("" : "=d" (x) : "0" (& SYM)); \ +if (x != VAL)\ + __builtin_abort(); \ + } while(0) + + +int main (void) +{ + CMP (g_1234, 1234); + CMP (w_4321, 4321); + CMP (l_5678, 5678); + + CMP (g_low, __AVR_SFR_OFFSET__ + 3); + CMP (w_low, __AVR_SFR_OFFSET__ + 2); + CMP (l_low, __AVR_SFR_OFFSET__ + 1); + + CMP (g_io, __AVR_SFR_OFFSET__ + 35); + CMP (w_io, __AVR_SFR_OFFSET__ + 34); + CMP (l_io, __AVR_SFR_OFFSET__ + 33); + + l_low = l_io; + g_low = g_io; + w_low = w_io; + l_low |= 1; + g_low |= 2; + w_low |= 4; + + return 0; +} + +/* { dg-final { scan-assembler "g_1234 = 1234" } } */ +/* { dg-final { scan-assembler "w_4321 = 4321" } } */ +/* { dg-final { scan-assembler "l_5678 = 5678" } } */ + +/* { dg-final { scan-assembler "\\.globl g_1234" } } */ +/* { dg-final { scan-assembler "\\.globl g_low" } } */ +/* { dg-final { scan-assembler "\\.globl g_io" } } */ + +/* { dg-final { scan-assembler "\\.weak w_4321" } } */ +/* { dg-final { scan-assembler "\\.weak w_low" } } */ +/* { dg-final { scan-assembler "\\.weak w_io" } } */ diff --git a/gcc/testsuite/gcc.target/avr/pr112952-0.c b/gcc/testsuite/gcc.target/avr/pr112952-0.c new file mode 100644 index 000..1870bf3f702 --- /dev/null +++ b/gcc/testsuite/gcc.target/avr/pr112952-0.c @@ -0,0 +1,16 @@ +/* { dg-do run } */ +/* { dg-options "-Os -save-temps -fno-data-sections -fno-common" } */ + +#include "attribute-io.h" + +/* { dg-final { scan-assembler "g_1234 = 1234" } } */ +/* { dg-final { scan-assembler "w_4321 = 4321" } } */ +/* { dg-final { scan-assembler "l_5678 = 5678" } } */ + +/* { dg-final { scan-assembler "\\.globl g_1234" } } */ +/* { dg-final { scan-assembler "\\.globl g_low" } } */ +/* { dg-final { scan-assembler "\\.globl g_io" } } */ + +/* { dg-final { scan-assembler "\\.weak w_4321" } } */ +/* { dg-final { scan-assembler "\\.weak w_low" } } */ +/* { dg-final { scan-assembler "\\.weak w_io" } } */ diff --git a/gcc/testsuite/gcc.target/avr/pr112952-1.c b/gcc/testsuite/gcc.target/avr/pr112952-1.c new file mode 100644 index 000..6e7d273c979 --- /dev/null +++ b/gcc/testsuite/gcc.target/avr/pr112952-1.c @@ -0,0 +1,16 @@ +/* { dg-do run } */ +/* { dg-options "-Os -save-temps -fno-data-sections -fcommon" } */ + +#include "attribute-io.h" + +/* { dg-final { scan-assembler "g_1234 = 1234" } } */ +/* { dg-final { scan-assembler "w_4321 = 4321" } } */ +/* { dg-final { scan-assembler "l_5678 = 5678" } } */ + +/* { dg-final { scan-assembler "\\.globl g_1234" } } */ +/* { dg-final { scan-assembler "\\.globl g_low" } } */ +/* { dg-final { scan-assembler "\\.globl g_io" } } */ + +/* { dg-final { scan-assembler "\\.weak w_4321" } } */ +/*