From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92264 invoked by alias); 3 Jun 2016 14:29:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 92244 invoked by uid 89); 3 Jun 2016 14:29:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:5708 X-HELO: eusmtp01.atmel.com Received: from eusmtp01.atmel.com (HELO eusmtp01.atmel.com) (212.144.249.243) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 03 Jun 2016 14:29:02 +0000 Received: from HNOCHT01.corp.atmel.com (10.161.30.161) by eusmtp01.atmel.com (10.161.101.31) with Microsoft SMTP Server (TLS) id 14.3.235.1; Fri, 3 Jun 2016 16:28:51 +0200 Received: from jaguar.atmel.com (10.161.30.18) by HNOCHT01.corp.atmel.com (10.161.30.161) with Microsoft SMTP Server (TLS) id 14.3.235.1; Fri, 3 Jun 2016 16:28:56 +0200 User-agent: mu4e 0.9.17; emacs 24.5.1 From: Senthil Kumar Selvaraj To: CC: Denis Chertykov , Georg-Johann Lay Subject: [Patch, avr] Fix PR 71151 Date: Fri, 03 Jun 2016 14:29:00 -0000 Message-ID: <87y46m1h3y.fsf@atmel.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2016-06/txt/msg00278.txt.bz2 Hi, This patch fixes PR 71151 by eliminating the TARGET_ASM_FUNCTION_RODATA_SECTION hook and setting JUMP_TABLES_IN_TEXT_SECTION to 1. As described in the bugzilla entry, this hook assumed it will get called only for jumptable rodata for functions. This was true until 6.1, when a commit in varasm.c started calling the hook for mergeable string/constant data as well. This resulted in string constants ending up in a section intended for jumptables (flash), and broke code using those constants, which expects them to be present in rodata (SRAM). Given that the original reason for placing jumptables in a section was fixed by Johann in PR 63323, this patch restores the original behavior. Reg testing on both gcc-6-branch and trunk showed no regressions. As pointed out by Johann, this may end up increasing code size if there are lots of branches that cross the jump tables. I intend to propose a separate patch that gives additional information to the target hook (SECCAT_RODATA_{STRING,JUMPTABLE}) so it can know what type of function rodata is coming on. Johann also suggested handling jump table generation ourselves - I'll experiment with that some more. If ok, could someone commit please? Could you also backport to gcc-6-branch? Regards Senthil gcc/ChangeLog 2016-06-03 Senthil Kumar Selvaraj * config/avr/avr.c (avr_asm_function_rodata_section): Remove. * config/avr/avr.c (TARGET_ASM_FUNCTION_RODATA_SECTION): Remove. gcc/testsuite/ChangeLog 2016-06-03 Senthil Kumar Selvaraj * gcc/testsuite/gcc.target/avr/pr71151-1.c: New. * gcc/testsuite/gcc.target/avr/pr71151-2.c: New. diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c index ba5cd91..3cb8cb7 100644 --- gcc/config/avr/avr.c +++ gcc/config/avr/avr.c @@ -9488,65 +9488,6 @@ avr_asm_init_sections (void) } -/* Implement `TARGET_ASM_FUNCTION_RODATA_SECTION'. */ - -static section* -avr_asm_function_rodata_section (tree decl) -{ - /* If a function is unused and optimized out by -ffunction-sections - and --gc-sections, ensure that the same will happen for its jump - tables by putting them into individual sections. */ - - unsigned int flags; - section * frodata; - - /* Get the frodata section from the default function in varasm.c - but treat function-associated data-like jump tables as code - rather than as user defined data. AVR has no constant pools. */ - { - int fdata = flag_data_sections; - - flag_data_sections = flag_function_sections; - frodata = default_function_rodata_section (decl); - flag_data_sections = fdata; - flags = frodata->common.flags; - } - - if (frodata != readonly_data_section - && flags & SECTION_NAMED) - { - /* Adjust section flags and replace section name prefix. */ - - unsigned int i; - - static const char* const prefix[] = - { - ".rodata", ".progmem.gcc_sw_table", - ".gnu.linkonce.r.", ".gnu.linkonce.t." - }; - - for (i = 0; i < sizeof (prefix) / sizeof (*prefix); i += 2) - { - const char * old_prefix = prefix[i]; - const char * new_prefix = prefix[i+1]; - const char * name = frodata->named.name; - - if (STR_PREFIX_P (name, old_prefix)) - { - const char *rname = ACONCAT ((new_prefix, - name + strlen (old_prefix), NULL)); - flags &= ~SECTION_CODE; - flags |= AVR_HAVE_JMP_CALL ? 0 : SECTION_CODE; - - return get_section (rname, flags, frodata->named.decl); - } - } - } - - return progmem_swtable_section; -} - - /* Implement `TARGET_ASM_NAMED_SECTION'. */ /* Track need of __do_clear_bss, __do_copy_data for named sections. */ @@ -13747,9 +13688,6 @@ avr_fold_builtin (tree fndecl, int n_args ATTRIBUTE_UNUSED, tree *arg, #undef TARGET_FOLD_BUILTIN #define TARGET_FOLD_BUILTIN avr_fold_builtin -#undef TARGET_ASM_FUNCTION_RODATA_SECTION -#define TARGET_ASM_FUNCTION_RODATA_SECTION avr_asm_function_rodata_section - #undef TARGET_SCALAR_MODE_SUPPORTED_P #define TARGET_SCALAR_MODE_SUPPORTED_P avr_scalar_mode_supported_p diff --git gcc/config/avr/avr.h gcc/config/avr/avr.h index 01da708..ab5e465 100644 --- gcc/config/avr/avr.h +++ gcc/config/avr/avr.h @@ -391,7 +391,7 @@ typedef struct avr_args #define SUPPORTS_INIT_PRIORITY 0 -#define JUMP_TABLES_IN_TEXT_SECTION 0 +#define JUMP_TABLES_IN_TEXT_SECTION 1 #define ASM_COMMENT_START " ; " diff --git gcc/testsuite/gcc.target/avr/pr71151-1.c gcc/testsuite/gcc.target/avr/pr71151-1.c new file mode 100644 index 0000000..615dce8 --- /dev/null +++ gcc/testsuite/gcc.target/avr/pr71151-1.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-Os -ffunction-sections -fdata-sections" } */ + +/* { dg-final { scan-assembler-not ".section .progmem.gcc_sw_table.foo.str1.1" } } */ +/* { dg-final { scan-assembler ".section .rodata.foo.str1.1,\"aMS\"" } } */ + + +extern void bar(const char*); +void foo(void) +{ + bar("BBBBBBBBBB"); +} diff --git gcc/testsuite/gcc.target/avr/pr71151-2.c gcc/testsuite/gcc.target/avr/pr71151-2.c new file mode 100644 index 0000000..0041858 --- /dev/null +++ gcc/testsuite/gcc.target/avr/pr71151-2.c @@ -0,0 +1,49 @@ +/* { dg-do run } */ +/* { dg-options "-Os -ffunction-sections -fdata-sections" } */ + +/* Make sure jumptables work properly, after removing the + special section placement hook. */ + +#include "exit-abort.h" + +volatile char y; +volatile char g; + +void foo(char x) +{ + switch (x) + { + case 0: + y = 67; break; + case 1: + y = 20; break; + case 2: + y = 109; break; + case 3: + y = 33; break; + case 4: + y = 44; break; + case 5: + y = 37; break; + case 6: + y = 10; break; + case 7: + y = 98; break; + } + y = y + g; +} + +int main() +{ + foo(5); + if (y != 37) + abort(); + + foo(0); + if (y != 67) + abort(); + + foo(7); + if (y != 98) + abort(); +} -- 2.7.4