From patchwork Wed Jul 20 14:27:02 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Georg-Johann Lay X-Patchwork-Id: 650704 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rvfPt1K5gz9t1N for ; Thu, 21 Jul 2016 00:27:29 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=t/MjQw09; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=vvKuK3VRppPnaqRsU oH5uoyM3+QRJuNkk0/D12qD5mGTWh/fD3Y8St+rdjqB/Iy7manTmxTawVHo/Z/UM hhZMsLwEHwaPmPv2+FWdDTL8sU1TROpgwzYbWJJUWm9URE23hc5G98sITqRqTTkZ Kn1yxhz8Pdt2G/wykA50r6nKkM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=wUkx2bZrctlalbuNWIm47Ak 6t+U=; b=t/MjQw09sF1xu1+MTfJsWKJGcJYFsDGHh9gWZ49X+6w+5TkrM8qWA/8 XAPPcc70wo+bMA99aVuHOZ/bLyd7IF1RihSyVtC77z0mweNzIR/618DKGSJugwsK iWq9n5jxYpeGrAHlcFWzVsiyodEuyGYO0n3LValq+2mOXrf60iIw= Received: (qmail 68493 invoked by alias); 20 Jul 2016 14:27:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 68482 invoked by uid 89); 20 Jul 2016 14:27:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.7 required=5.0 tests=AWL, BAYES_50, KAM_ASCII_DIVIDERS, KAM_STOCKGEN, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=hallo, resort, locates, UD:avr.c X-HELO: mo4-p00-ob.smtp.rzone.de Received: from mo4-p00-ob.smtp.rzone.de (HELO mo4-p00-ob.smtp.rzone.de) (81.169.146.162) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 20 Jul 2016 14:27:11 +0000 X-RZG-AUTH: :LXoWVUeid/7A29J/hMvvT3ol15ykJcYwR/bcHRirORRW3yMcVao= X-RZG-CLASS-ID: mo00 Received: from [192.168.0.123] (mail.hightec-rt.com [213.135.1.215]) by smtp.strato.de (RZmta 38.13 DYNA|AUTH) with ESMTPSA id V0a1d0s6KER2pqI (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Wed, 20 Jul 2016 16:27:02 +0200 (CEST) Subject: Re: [patch, avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE To: Denis Chertykov References: <051d5d94-6175-0b11-04ee-439f0cab870a@gjlay.de> <174660de-89bb-5727-7fcd-64d4cf5a5aba@gjlay.de> Cc: gcc-patches , Senthil Kumar Selvaraj From: Georg-Johann Lay Message-ID: Date: Wed, 20 Jul 2016 16:27:02 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes 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: doc/extend.texi =================================================================== --- doc/extend.texi (revision 238524) +++ doc/extend.texi (revision 238525) @@ -1422,6 +1422,11 @@ const __memx void *pfoo = &foo; Such code requires at least binutils 2.23, see @w{@uref{http://sourceware.org/PR13503,PR13503}}. +@item +On the reduced Tiny devices like ATtiny40, no address spaces are supported. +Data can be put into and read from flash memory by means of +attribute @code{progmem}, see @ref{AVR Variable Attributes}. + @end itemize @subsection M32C Named Address Spaces @@ -5847,10 +5852,12 @@ attribute accomplishes this by putting r section whose name starts with @code{.progmem}. This attribute works similar to the @code{section} attribute -but adds additional checking. Notice that just like the -@code{section} attribute, @code{progmem} affects the location -of the data but not how this data is accessed. +but adds additional checking. +@table @asis +@item @bullet{}@tie{} Ordinary AVR cores with 32 general purpose registers: +@code{progmem} affects the location +of the data but not how this data is accessed. In order to read data located with the @code{progmem} attribute (inline) assembler must be used. @smallexample @@ -5873,6 +5880,28 @@ normally resides in the data memory (RAM See also the @ref{AVR Named Address Spaces} section for an alternate way to locate and access data in flash memory. +@item @bullet{}@tie{}Reduced AVR Tiny cores like ATtiny40: +The compiler adds @code{0x4000} +to the addresses of objects and declarations in @code{progmem} and locates +the objects in flash memory, namely in section @code{.progmem.data}. +The offset is needed because the flash memory is visible in the RAM +address space starting at address @code{0x4000}. + +Data in @code{progmem} can be accessed by means of ordinary C@tie{}code, +no special functions or macros are needed. + +@smallexample +/* var is located in flash memory */ +extern const int var[2] __attribute__((progmem)); + +int read_var (int i) +@{ + return var[i]; +@} +@end smallexample + +@end table + @item io @itemx io (@var{addr}) @cindex @code{io} variable attribute, AVR Index: testsuite/gcc.target/avr/torture/tiny-progmem.c =================================================================== --- testsuite/gcc.target/avr/torture/tiny-progmem.c (nonexistent) +++ testsuite/gcc.target/avr/torture/tiny-progmem.c (revision 238525) @@ -0,0 +1,109 @@ +/* { dg-do run } */ +/* { dg-options "-Wl,--defsym,test6_xdata=0" } */ + +#ifdef __AVR_TINY__ +#define PM __attribute__((__progmem__)) +#else +/* On general core, just resort to vanilla C. */ +#define PM /* Empty */ +#endif + +#define PSTR(s) (__extension__({ static const char __c[] PM = (s); &__c[0];})) + +#define NI __attribute__((noinline,noclone)) + +const volatile int data[] PM = { 1234, 5678 }; +const volatile int * volatile pdata = &data[1]; + +int ram[2]; + +const int myvar PM = 42; +extern const int xvar __asm ("myvar") PM; + +NI int const volatile* get_addr_1 (void) +{ + return &data[1]; +} + +NI int const volatile* get_addr_x (int x) +{ + return &data[x]; +} + +void test_1 (void) +{ + if (data[0] != 1234) + __builtin_abort(); + + if (data[1] != 5678) + __builtin_abort(); +} + +void test_2 (void) +{ + if (data[1] != 5678) + __builtin_abort(); +} + +void test_3 (void) +{ + if (&data[1] != pdata) + __builtin_abort(); +} + +void test_4 (void) +{ + if (5678 != *get_addr_1()) + __builtin_abort(); + if (5678 != *get_addr_x(1)) + __builtin_abort(); +} + +void test_5 (void) +{ + __builtin_memcpy (&ram, (void*) &data, 4); + if (ram[0] - ram[1] != 1234 - 5678) + __builtin_abort(); +} + +const char pmSTR[] PM = "01234"; + +NI const char* get_pmSTR (int i) +{ + return pmSTR + 2 + i; +} + +void test_6 (void) +{ +#ifdef __AVR_TINY__ + extern const int test6_xdata PM; + const char* str = PSTR ("Hallo"); + if (0 == (__AVR_TINY_PM_BASE_ADDRESS__ & (__UINTPTR_TYPE__) str)) + __builtin_abort(); + if (0 == (__AVR_TINY_PM_BASE_ADDRESS__ & (__UINTPTR_TYPE__) test6_xdata)) + __builtin_abort(); +#endif + + if (get_pmSTR (0)[0] != '0' + 2) + __builtin_abort(); + if (get_pmSTR (1)[0] != '1' + 2) + __builtin_abort(); +} + +void test_7 (void) +{ + if (xvar != 42) + __builtin_abort(); +} + +int main() +{ + test_1(); + test_2(); + test_3(); + test_4(); + test_5(); + test_6(); + test_7(); + return 0; +} Index: config/avr/avr-c.c =================================================================== --- config/avr/avr-c.c (revision 238524) +++ config/avr/avr-c.c (revision 238525) @@ -296,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); @@ -337,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) Index: config/avr/avr.c =================================================================== --- config/avr/avr.c (revision 238524) +++ config/avr/avr.c (revision 238525) @@ -80,6 +80,10 @@ ((SYMBOL_REF_FLAGS (sym) & AVR_SYMBOL_FLAG_PROGMEM) \ / SYMBOL_FLAG_MACH_DEP) +/* (AVR_TINY only): Symbol has attribute progmem */ +#define AVR_SYMBOL_FLAG_TINY_PM \ + (SYMBOL_FLAG_MACH_DEP << 4) + #define TINY_ADIW(REG1, REG2, I) \ "subi " #REG1 ",lo8(-(" #I "))" CR_TAB \ "sbci " #REG2 ",hi8(-(" #I "))" @@ -2161,12 +2165,35 @@ cond_string (enum rtx_code code) } +/* Return true if rtx X is a CONST or SYMBOL_REF with progmem. + This must be used for AVR_TINY only because on other cores + the flash memory is not visible in the RAM address range and + cannot be read by, say, LD instruction. */ + +static bool +avr_address_tiny_pm_p (rtx x) +{ + if (CONST == GET_CODE (x)) + x = XEXP (XEXP (x, 0), 0); + + if (SYMBOL_REF_P (x)) + return SYMBOL_REF_FLAGS (x) & AVR_SYMBOL_FLAG_TINY_PM; + + return false; +} + /* Implement `TARGET_PRINT_OPERAND_ADDRESS'. */ /* Output ADDR to FILE as address. */ static void avr_print_operand_address (FILE *file, machine_mode /*mode*/, rtx addr) { + if (AVR_TINY + && avr_address_tiny_pm_p (addr)) + { + addr = plus_constant (Pmode, addr, AVR_TINY_PM_OFFSET); + } + switch (GET_CODE (addr)) { case REG: @@ -8937,6 +8964,12 @@ avr_assemble_integer (rtx x, unsigned in return true; } + if (AVR_TINY + && avr_address_tiny_pm_p (x)) + { + x = plus_constant (Pmode, x, AVR_TINY_PM_OFFSET); + } + return default_assemble_integer (x, size, aligned_p); } @@ -9603,7 +9636,7 @@ avr_encode_section_info (tree decl, rtx if (decl && DECL_P (decl) && TREE_CODE (decl) != FUNCTION_DECL && MEM_P (rtl) - && SYMBOL_REF == GET_CODE (XEXP (rtl, 0))) + && SYMBOL_REF_P (XEXP (rtl, 0))) { rtx sym = XEXP (rtl, 0); tree type = TREE_TYPE (decl); @@ -9616,7 +9649,8 @@ avr_encode_section_info (tree decl, rtx /* PSTR strings are in generic space but located in flash: patch address space. */ - if (-1 == avr_progmem_p (decl, attr)) + if (!AVR_TINY + && -1 == avr_progmem_p (decl, attr)) as = ADDR_SPACE_FLASH; AVR_SYMBOL_SET_ADDR_SPACE (sym, as); @@ -9647,6 +9681,19 @@ avr_encode_section_info (tree decl, rtx if (addr_attr && !DECL_EXTERNAL (decl)) SYMBOL_REF_FLAGS (sym) |= SYMBOL_FLAG_ADDRESS; } + + if (AVR_TINY + && decl + && VAR_DECL == TREE_CODE (decl) + && -1 == avr_progmem_p (decl, DECL_ATTRIBUTES (decl)) + && MEM_P (rtl) + && SYMBOL_REF_P (XEXP (rtl, 0))) + { + /* Tag symbols for later addition of 0x4000 (AVR_TINY_PM_OFFSET). */ + + rtx sym = XEXP (rtl, 0); + SYMBOL_REF_FLAGS (sym) |= AVR_SYMBOL_FLAG_TINY_PM; + } } Index: config/avr/avr.h =================================================================== --- config/avr/avr.h (revision 238524) +++ config/avr/avr.h (revision 238525) @@ -74,6 +74,8 @@ enum || avr_arch->have_rampd) #define AVR_HAVE_EIJMP_EICALL (avr_arch->have_eijmp_eicall) +#define AVR_TINY_PM_OFFSET (0x4000) + /* Handling of 8-bit SP versus 16-bit SP is as follows: FIXME: DRIVER_SELF_SPECS has changed.