Re: [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE

2016-07-20 Thread Georg-Johann Lay 

On 18.07.2016 08:58, Denis Chertykov wrote:

2016-07-15 18:26 GMT+03:00 Georg-Johann Lay :

This patch needs new hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE:
https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00839.html

This patch turns attribute progmem into a working feature for AVR_TINY
cores.

It boils down to adding 0x4000 to all symbols with progmem:  Flash memory
can be seen in the RAM address space starting at 0x4000, i.e. data in flash
can be read by means of LD instruction if we add offsets of 0x4000.  There
is no need for special access macros like pgm_read_* or special address
spaces as there is nothing like a LPM instruction.

This is simply achieved by setting a respective symbol_ref_flag, and when
such a symbol has to be printed, then plus_constant with 0x4000 is used.

Diagnosing of unsupported address spaces is now performed by
TARGET_ADDR_SPACE_DIAGNOSE_USAGE which has exact location information.
Hence there is no need to scan all decls for invalid address spaces.

For AVR_TINY, alls address spaces have been disabled.  They are of no use.
Supporting __flash would just make the backend more complicated without any
gains.


Ok for trunk?

Johann


gcc/
* doc/extend.texi (AVR Variable Attributes) [progmem]: Add
documentation how it works on reduced Tiny cores.
(AVR Named Address Spaces): No support for reduced Tiny.
* avr-protos.h (avr_addr_space_supported_p): New prototype.
* avr.c (AVR_SYMBOL_FLAG_TINY_PM): New macro.
(avr_address_tiny_pm_p): New static function.
(avr_print_operand_address) [AVR_TINY]: Add AVR_TINY_PM_OFFSET
if the address is in progmem.
(avr_assemble_integer): Same.
(avr_encode_section_info) [AVR_TINY]: Set AVR_SYMBOL_FLAG_TINY_PM
for symbol_ref in progmem.
(TARGET_ADDR_SPACE_DIAGNOSE_USAGE): New hook define...
(avr_addr_space_diagnose_usage): ...and implementation.
(avr_addr_space_supported_p): New function.
(avr_nonconst_pointer_addrspace, avr_pgm_check_var_decl): Only
report bad address space usage if that space is supported.
(avr_insert_attributes): Same.  No more complain about unsupported
address spaces.
* avr.h (AVR_TINY_PM_OFFSET): New macro.
* avr-c.c (tm_p.h): Include it.
(avr_cpu_cpp_builtins) [__AVR_TINY_PM_BASE_ADDRESS__]: Use
AVR_TINY_PM_OFFSET instead of magic 0x4000 when built-in def'ing.
Only define addr-space related built-in macro if
avr_addr_space_supported_p.
gcc/testsuite/
* gcc.target/avr/torture/tiny-progmem.c: New test.



Approved.


Committed, but I split it into 2 change-sets.  The only effective change is 
that the hook has a different prototype (returns void instead of bool).



Part1: Implement new target hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE.

https://gcc.gnu.org/r238519

gcc/
* avr-protos.h (avr_addr_space_supported_p): New prototype.
* avr.c (TARGET_ADDR_SPACE_DIAGNOSE_USAGE): New hook define...
(avr_addr_space_diagnose_usage): ...and implementation.
(avr_addr_space_supported_p): New function.
(avr_nonconst_pointer_addrspace, avr_pgm_check_var_decl): Only
report bad address space usage if that space is supported.
(avr_insert_attributes): Same.  No more complain about unsupported
address spaces.
* avr-c.c (tm_p.h): Include it.
(avr_cpu_cpp_builtins): Only define addr-space related built-in
macro if avr_addr_space_supported_p.

Part2: Make progmem work for reduced Tiny cores

https://gcc.gnu.org/r238525

gcc/
Implement attribute progmem on reduced Tiny cores by adding
flash offset 0x4000 to respective symbols.

PR target/71948
* doc/extend.texi (AVR Variable Attributes) [progmem]: Add
documentation how it works on reduced Tiny cores.
(AVR Named Address Spaces): No support for reduced Tiny.
* config/avr/avr.c (AVR_SYMBOL_FLAG_TINY_PM): New macro.
(avr_address_tiny_pm_p): New static function.
(avr_print_operand_address) [AVR_TINY]: Add AVR_TINY_PM_OFFSET
if the address is in progmem.
(avr_assemble_integer): Same.
(avr_encode_section_info) [AVR_TINY]: Set AVR_SYMBOL_FLAG_TINY_PM
for symbol_ref in progmem.
* config/avr/avr.h (AVR_TINY_PM_OFFSET): New macro.
* config/avr/avr-c.c (avr_cpu_cpp_builtins): Use it instead of
magic 0x4000 when built-in def'ing __AVR_TINY_PM_BASE_ADDRESS__.
gcc/testsuite/
PR target/71948
* gcc.target/avr/torture/tiny-progmem.c: New test.

Index: config/avr/avr-c.c
===
--- config/avr/avr-c.c	(revision 238518)
+++ config/avr/avr-c.c	(revision 238519)
@@ -26,7 +26,7 @@
 #include "c-family/c-common.h"
 #include "stor-layout.h"
 #include "langhooks.h"
-
+#include "tm_p.h"
 
 /* IDs for all the AVR builtins.  */
 
@@ -253,7 +253,10

Re: [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE

2016-07-18 Thread Denis Chertykov 
2016-07-15 18:26 GMT+03:00 Georg-Johann Lay :
> This patch needs new hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE:
> https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00839.html
>
> This patch turns attribute progmem into a working feature for AVR_TINY
> cores.
>
> It boils down to adding 0x4000 to all symbols with progmem:  Flash memory
> can be seen in the RAM address space starting at 0x4000, i.e. data in flash
> can be read by means of LD instruction if we add offsets of 0x4000.  There
> is no need for special access macros like pgm_read_* or special address
> spaces as there is nothing like a LPM instruction.
>
> This is simply achieved by setting a respective symbol_ref_flag, and when
> such a symbol has to be printed, then plus_constant with 0x4000 is used.
>
> Diagnosing of unsupported address spaces is now performed by
> TARGET_ADDR_SPACE_DIAGNOSE_USAGE which has exact location information.
> Hence there is no need to scan all decls for invalid address spaces.
>
> For AVR_TINY, alls address spaces have been disabled.  They are of no use.
> Supporting __flash would just make the backend more complicated without any
> gains.
>
>
> Ok for trunk?
>
> Johann
>
>
> gcc/
> * doc/extend.texi (AVR Variable Attributes) [progmem]: Add
> documentation how it works on reduced Tiny cores.
> (AVR Named Address Spaces): No support for reduced Tiny.
> * avr-protos.h (avr_addr_space_supported_p): New prototype.
> * avr.c (AVR_SYMBOL_FLAG_TINY_PM): New macro.
> (avr_address_tiny_pm_p): New static function.
> (avr_print_operand_address) [AVR_TINY]: Add AVR_TINY_PM_OFFSET
> if the address is in progmem.
> (avr_assemble_integer): Same.
> (avr_encode_section_info) [AVR_TINY]: Set AVR_SYMBOL_FLAG_TINY_PM
> for symbol_ref in progmem.
> (TARGET_ADDR_SPACE_DIAGNOSE_USAGE): New hook define...
> (avr_addr_space_diagnose_usage): ...and implementation.
> (avr_addr_space_supported_p): New function.
> (avr_nonconst_pointer_addrspace, avr_pgm_check_var_decl): Only
> report bad address space usage if that space is supported.
> (avr_insert_attributes): Same.  No more complain about unsupported
> address spaces.
> * avr.h (AVR_TINY_PM_OFFSET): New macro.
> * avr-c.c (tm_p.h): Include it.
> (avr_cpu_cpp_builtins) [__AVR_TINY_PM_BASE_ADDRESS__]: Use
> AVR_TINY_PM_OFFSET instead of magic 0x4000 when built-in def'ing.
> Only define addr-space related built-in macro if
> avr_addr_space_supported_p.
> gcc/testsuite/
> * gcc.target/avr/torture/tiny-progmem.c: New test.
>

Approved.

[patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE

2016-07-15 Thread Georg-Johann Lay 

This patch needs new hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE:
https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00839.html

This patch turns attribute progmem into a working feature for AVR_TINY cores.

It boils down to adding 0x4000 to all symbols with progmem:  Flash memory can 
be seen in the RAM address space starting at 0x4000, i.e. data in flash can be 
read by means of LD instruction if we add offsets of 0x4000.  There is no need 
for special access macros like pgm_read_* or special address spaces as there is 
nothing like a LPM instruction.


This is simply achieved by setting a respective symbol_ref_flag, and when such 
a symbol has to be printed, then plus_constant with 0x4000 is used.


Diagnosing of unsupported address spaces is now performed by 
TARGET_ADDR_SPACE_DIAGNOSE_USAGE which has exact location information.  Hence 
there is no need to scan all decls for invalid address spaces.


For AVR_TINY, alls address spaces have been disabled.  They are of no use. 
Supporting __flash would just make the backend more complicated without any gains.



Ok for trunk?

Johann


gcc/
* doc/extend.texi (AVR Variable Attributes) [progmem]: Add
documentation how it works on reduced Tiny cores.
(AVR Named Address Spaces): No support for reduced Tiny.
* avr-protos.h (avr_addr_space_supported_p): New prototype.
* avr.c (AVR_SYMBOL_FLAG_TINY_PM): New macro.
(avr_address_tiny_pm_p): New static function.
(avr_print_operand_address) [AVR_TINY]: Add AVR_TINY_PM_OFFSET
if the address is in progmem.
(avr_assemble_integer): Same.
(avr_encode_section_info) [AVR_TINY]: Set AVR_SYMBOL_FLAG_TINY_PM
for symbol_ref in progmem.
(TARGET_ADDR_SPACE_DIAGNOSE_USAGE): New hook define...
(avr_addr_space_diagnose_usage): ...and implementation.
(avr_addr_space_supported_p): New function.
(avr_nonconst_pointer_addrspace, avr_pgm_check_var_decl): Only
report bad address space usage if that space is supported.
(avr_insert_attributes): Same.  No more complain about unsupported
address spaces.
* avr.h (AVR_TINY_PM_OFFSET): New macro.
* avr-c.c (tm_p.h): Include it.
(avr_cpu_cpp_builtins) [__AVR_TINY_PM_BASE_ADDRESS__]: Use
AVR_TINY_PM_OFFSET instead of magic 0x4000 when built-in def'ing.
Only define addr-space related built-in macro if
avr_addr_space_supported_p.
gcc/testsuite/
* gcc.target/avr/torture/tiny-progmem.c: New test.

Index: config/avr/avr-c.c
===
--- config/avr/avr-c.c	(revision 237643)
+++ config/avr/avr-c.c	(working copy)
@@ -26,7 +26,7 @@
 #include "c-family/c-common.h"
 #include "stor-layout.h"
 #include "langhooks.h"
-
+#include "tm_p.h"
 
 /* IDs for all the AVR builtins.  */
 
@@ -253,7 +253,10 @@ avr_register_target_pragmas (void)
   gcc_assert (ADDR_SPACE_GENERIC == ADDR_SPACE_RAM);
 
   /* Register address spaces.  The order must be the same as in the respective
- enum from avr.h (or designated initializers must be used in avr.c).  */
+ enum from avr.h (or designated initializers must be used in avr.c).
+ We always register all address spaces even if some of them make no
+ sense for some targets.  Diagnose for non-supported spaces will be
+ emit by TARGET_ADDR_SPACE_DIAGNOSE_USAGE.  */
 
   for (i = 0; i < ADDR_SPACE_COUNT; i++)
 {
@@ -293,7 +296,7 @@ avr_cpu_cpp_builtins (struct cpp_reader
   builtin_define_std ("AVR");
 
   /* __AVR_DEVICE_NAME__ and  avr_mcu_types[].macro like __AVR_ATmega8__
-	 are defined by -D command option, see device-specs file.  */
+ are defined by -D command option, see device-specs file.  */
 
   if (avr_arch->macro)
 cpp_define_formatted (pfile, "__AVR_ARCH__=%s", avr_arch->macro);
@@ -334,7 +337,8 @@ start address.  This macro shall be used
  it has been mapped to the data memory.  For AVR_TINY devices
  (ATtiny4/5/9/10/20 and 40) mapped program memory starts at 0x4000. */
 
-  cpp_define (pfile, "__AVR_TINY_PM_BASE_ADDRESS__=0x4000");
+  cpp_define_formatted (pfile, "__AVR_TINY_PM_BASE_ADDRESS__=0x%x",
+AVR_TINY_PM_OFFSET);
 }
 
   if (AVR_HAVE_EIJMP_EICALL)
@@ -391,10 +395,7 @@ /* Define builtin macros so that the use
 /* Only supply __FLASH macro if the address space is reasonable
for this target.  The address space qualifier itself is still
supported, but using it will throw an error.  */
-&& avr_addrspace[i].segment < avr_n_flash
-	/* Only support __MEMX macro if we have LPM.  */
-	&& (AVR_HAVE_LPM || avr_addrspace[i].pointer_size <= 2))
-
+&& avr_addr_space_supported_p ((addr_space_t) i))
   {
 const char *name = avr_addrspace[i].name;
 char *Name = (char*) alloca (1 + strlen (name));
Index: config/avr/avr-protos.h