From patchwork Wed Jul 5 13:28:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Georg-Johann Lay X-Patchwork-Id: 784607 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 3x2hXs2fXFz9s3w for ; Wed, 5 Jul 2017 23:28:59 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="SkYaty82"; 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:to:cc :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=wwEWTYii7wAJx16dpSnS54qZIhZhG6Kmcq3lYnieek3b68/mD+ VHc1LJa1AnykqvIEmFyh8coXygUrZDM9Rv/pJRXZqrE+ox/p56BjPvYXutSj0Ek4 FD9CBZqVyQmeIJIMPx9R45RoKMSw3U9TuSK16cDE9lu/UZ+f3BXe837oU= 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:to:cc :from:subject:message-id:date:mime-version:content-type; s= default; bh=nNjY1NrJYv4p5q2z9jcR8N/2Kjs=; b=SkYaty82qShWkKGBrd01 u0l0vBUPL/lwm+TnNWTbzVgaDbeGX5W6fBEWfZLQy6kDatU3II2TeSQZ412VaeDi liJgQs9FGJaD5dy/mA6WTagvBij3fhNh4qvDKPcpfzy7kdk4SmDS79SR/pvYxLv3 VyBHMC/Q07AIzJvmg3OzFxA= Received: (qmail 19560 invoked by alias); 5 Jul 2017 13:28:48 -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 18864 invoked by uid 89); 5 Jul 2017 13:28:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=0m, 10m, 1m, 0M 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.221) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Jul 2017 13:28:44 +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 41.1 DYNA|AUTH) with ESMTPSA id j0aa5bt65DSfAdj (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Wed, 5 Jul 2017 15:28:41 +0200 (CEST) To: gcc-patches Cc: Denis Chertykov From: Georg-Johann Lay Subject: [patch,avr,committed] Fix PR81305 Message-ID: <9f3af784-cbdf-f3dd-385c-7296f543c87a@gjlay.de> Date: Wed, 5 Jul 2017 15:28:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 X-IsSubscribed: yes Hi, Instruction selection must not depend on "optimize" because LDS / STS range might not cover range of IN / OUT. This lead to wrong ISR code for avrtiny. Applied as obvious. Also added some test coverage for ISRs which we didn't have at all to date. Johann gcc/ PR target/81305 * config/avr/avr.c (avr_out_movhi_mr_r_xmega) [CONSTANT_ADDRESS_P]: Don't depend on "optimize > 0". (out_movhi_r_mr, out_movqi_mr_r): Same. (out_movhi_mr_r, out_movqi_r_mr): Same. (avr_address_cost) [CONSTANT_ADDRESS_P]: Don't depend cost for io_address_operand on "optimize > 0". gcc/testsuite/ PR target/81305 * gcc.target/avr/isr-test.h: New file. * gcc.target/avr/torture/isr-01-simple.c: New test. * gcc.target/avr/torture/isr-02-call.c: New test. * gcc.target/avr/torture/isr-03-fixed.c: New test. Index: config/avr/avr.c =================================================================== --- config/avr/avr.c (revision 249997) +++ config/avr/avr.c (revision 249998) @@ -3820,7 +3820,7 @@ out_movqi_r_mr (rtx_insn *insn, rtx op[] if (CONSTANT_ADDRESS_P (x)) { int n_words = AVR_TINY ? 1 : 2; - return optimize > 0 && io_address_operand (x, QImode) + return io_address_operand (x, QImode) ? avr_asm_len ("in %0,%i1", op, plen, -1) : avr_asm_len ("lds %0,%m1", op, plen, -n_words); } @@ -4088,7 +4088,7 @@ out_movhi_r_mr (rtx_insn *insn, rtx op[] else if (CONSTANT_ADDRESS_P (base)) { int n_words = AVR_TINY ? 2 : 4; - return optimize > 0 && io_address_operand (base, HImode) + return io_address_operand (base, HImode) ? avr_asm_len ("in %A0,%i1" CR_TAB "in %B0,%i1+1", op, plen, -2) @@ -5215,7 +5215,7 @@ out_movqi_mr_r (rtx_insn *insn, rtx op[] if (CONSTANT_ADDRESS_P (x)) { int n_words = AVR_TINY ? 1 : 2; - return optimize > 0 && io_address_operand (x, QImode) + return io_address_operand (x, QImode) ? avr_asm_len ("out %i0,%1", op, plen, -1) : avr_asm_len ("sts %m0,%1", op, plen, -n_words); } @@ -5291,13 +5291,12 @@ avr_out_movhi_mr_r_xmega (rtx_insn *insn if (CONSTANT_ADDRESS_P (base)) { - int n_words = AVR_TINY ? 2 : 4; - return optimize > 0 && io_address_operand (base, HImode) + return io_address_operand (base, HImode) ? avr_asm_len ("out %i0,%A1" CR_TAB "out %i0+1,%B1", op, plen, -2) : avr_asm_len ("sts %m0,%A1" CR_TAB - "sts %m0+1,%B1", op, plen, -n_words); + "sts %m0+1,%B1", op, plen, -4); } if (reg_base > 0) @@ -5477,7 +5476,7 @@ out_movhi_mr_r (rtx_insn *insn, rtx op[] if (CONSTANT_ADDRESS_P (base)) { int n_words = AVR_TINY ? 2 : 4; - return optimize > 0 && io_address_operand (base, HImode) + return io_address_operand (base, HImode) ? avr_asm_len ("out %i0+1,%B1" CR_TAB "out %i0,%A1", op, plen, -2) @@ -11361,8 +11360,7 @@ avr_address_cost (rtx x, machine_mode mo } else if (CONSTANT_ADDRESS_P (x)) { - if (optimize > 0 - && io_address_operand (x, QImode)) + if (io_address_operand (x, QImode)) cost = 2; if (AVR_TINY Index: testsuite/gcc.target/avr/isr-test.h =================================================================== --- testsuite/gcc.target/avr/isr-test.h (nonexistent) +++ testsuite/gcc.target/avr/isr-test.h (revision 249998) @@ -0,0 +1,282 @@ +#ifndef ISR_TEST_H +#define ISR_TEST_H + +#include + +#define ISR(N,...) \ +__attribute__ ((used, externally_visible , ## __VA_ARGS__)) \ + void __vector_##N (void); \ + void __vector_##N (void) + +#define SFR(ADDR) (*(unsigned char volatile*) (__AVR_SFR_OFFSET__ + (ADDR))) +#define CORE_SFRS SFR (0x38) +#define SREG SFR (0x3F) +#define SPL SFR (0x3D) +#define EIND SFR (0x3C) +#define RAMPZ SFR (0x3B) +#define RAMPY SFR (0x3A) +#define RAMPX SFR (0x39) +#define RAMPD SFR (0x38) + +#ifdef __AVR_HAVE_JMP_CALL__ +#define VEC_SIZE 4 +#else +#define VEC_SIZE 2 +#endif + +#ifdef __AVR_TINY__ +#define FIRST_REG 16 +#else +#define FIRST_REG 0 +#endif + +#define CR "\n\t" + +typedef struct +{ + unsigned char sfrs[8]; + unsigned char gprs[32 - FIRST_REG]; +} regs_t; + +regs_t reginfo1, reginfo2; + +__attribute__((noinline)) +static void clear_reginfo (void) +{ + memset (reginfo1.sfrs, 0, sizeof (reginfo1.sfrs)); + memset (reginfo2.sfrs, 0, sizeof (reginfo2.sfrs)); +} + +__attribute__((noinline)) +static void compare_reginfo (unsigned long gpr_ignore) +{ + signed char regno; + const unsigned char *preg1 = ®info1.gprs[0]; + const unsigned char *preg2 = ®info2.gprs[0]; + + if (memcmp (®info1, ®info2, 8)) + __builtin_abort(); + + gpr_ignore >>= FIRST_REG; + + for (regno = FIRST_REG; regno < 32; + regno++, preg1++, preg2++, gpr_ignore >>= 1) + { + if (gpr_ignore & 1) + continue; + + if (*preg1 != *preg2) + { + static signed char volatile failed_regno; + failed_regno = regno; + __builtin_abort(); + } + } +} + +/* STore GPR */ +#define ST(regno,M) \ + CR "sts %[" #M "]+8-%[first]+" #regno ", r" #regno + +/* STore SFR */ +#define ST_SFR(sfr, n_sfr, M) \ + CR "in __tmp_reg__,%i[s_" #sfr "]" \ + CR "sts %[" #M "]+" #n_sfr ", __tmp_reg__" + +/* Named asm OPerand for SFR */ +#define OP_SFR(sfr) \ + , [s_ ## sfr] "n" (&(sfr)) + +/* Write funny value to SFR */ +#define XX_SFR(sfr) \ + CR "dec r31 $ out %i[s_" #sfr "], r31" + +/* Write 0 to SFR */ +#define OO_SFR(sfr) \ + CR "out %i[s_" #sfr "], __zero_reg__" + +/* Macros for SREG */ +#define ST_SREG(M) ST_SFR (SREG,0,M) +#define OP_SREG OP_SFR (SREG) +#define XX_SREG XX_SFR (SREG) + +/* Macros for EIND */ +#if defined __AVR_HAVE_EIJMP_EICALL__ +#define ST_EIND(M) ST_SFR (EIND,1,M) +#define OP_EIND OP_SFR (EIND) +#else +#define ST_EIND(M) /* empty */ +#define OP_EIND /* empty */ +#endif + +/* Macros for RAMPX */ +#if defined (__AVR_HAVE_RAMPX__) +#define ST_RAMPX(M) ST_SFR (RAMPX,2,M) +#define OP_RAMPX OP_SFR (RAMPX) +#define XX_RAMPX XX_SFR (RAMPX) +#define OO_RAMPX OO_SFR (RAMPX) +#else +#define ST_RAMPX(M) /* empty */ +#define OP_RAMPX /* empty */ +#define XX_RAMPX /* empty */ +#define OO_RAMPX /* empty */ +#endif + +/* Macros for RAMPY */ +#if defined (__AVR_HAVE_RAMPY__) +#define ST_RAMPY(M) ST_SFR (RAMPY,3,M) +#define OP_RAMPY OP_SFR (RAMPY) +#define XX_RAMPY XX_SFR (RAMPY) +#define OO_RAMPY OO_SFR (RAMPY) +#else +#define ST_RAMPY(M) /* empty */ +#define OP_RAMPY /* empty */ +#define XX_RAMPY /* empty */ +#define OO_RAMPY /* empty */ +#endif + +/* Macros for RAMPZ */ +#if defined (__AVR_HAVE_RAMPZ__) +#define ST_RAMPZ(M) ST_SFR (RAMPZ,4,M) +#define OP_RAMPZ OP_SFR (RAMPZ) +#define XX_RAMPZ XX_SFR (RAMPZ) +#define OO_RAMPZ OO_SFR (RAMPZ) +#else +#define ST_RAMPZ(M) /* empty */ +#define OP_RAMPZ /* empty */ +#define XX_RAMPZ /* empty */ +#define OO_RAMPZ /* empty */ +#endif + +/* Macros for RAMPD */ +#if defined (__AVR_HAVE_RAMPD__) +#define ST_RAMPD(M) ST_SFR (RAMPD,5,M) +#define OP_RAMPD OP_SFR (RAMPD) +#else +#define ST_RAMPD(M) /* empty */ +#define OP_RAMPD /* empty */ +#endif + +/* Macros for all GPRs */ +#if defined __AVR_TINY__ +#define ST_REGS_LO(M) /* empty */ +#else +#define ST_REGS_LO(M) \ + ST(0,M) ST(1,M) ST(2,M) ST(3,M) \ + ST(4,M) ST(5,M) ST(6,M) ST(7,M) \ + ST(8,M) ST(9,M) ST(10,M) ST(11,M) \ + ST(12,M) ST(13,M) ST(14,M) ST(15,M) +#endif /* AVR_TINY */ + +#define ST_REGS_HI(M) \ + ST(16,M) ST(17,M) ST(18,M) ST(19,M) \ + ST(20,M) ST(21,M) ST(22,M) ST(23,M) \ + ST(24,M) ST(25,M) ST(26,M) ST(27,M) \ + ST(28,M) ST(29,M) ST(30,M) ST(31,M) + +__attribute__((used,naked,noinline,noclone)) +static void host_store1 (void) +{ + __asm __volatile__ + ("nop" + CR ".global do_stores_before" + CR ".type do_stores_before,@function" + CR "do_stores_before:" + /* Funny values to some SFRs */ + CR "ldi r31, 1 + 'Z'" + XX_RAMPZ + XX_RAMPY + XX_RAMPX + CR "dec __zero_reg__" + CR "clr r31" + XX_SREG + /* Must set I-flag due to RETI of ISR */ + CR "sei" + /* Store core regs before ISR */ + ST_RAMPX (mem1) + ST_RAMPY (mem1) + ST_RAMPZ (mem1) + ST_RAMPD (mem1) + ST_EIND (mem1) + ST_SREG (mem1) + CR "ldi r31, 0xaa" + CR "mov __tmp_reg__, r31" + CR "ldi r31, 31" + ST_REGS_LO (mem1) + ST_REGS_HI (mem1) + CR "ret" + : /* No outputs */ + : [mem1] "i" (®info1), [first] "n" (FIRST_REG) + OP_RAMPX + OP_RAMPY + OP_RAMPZ + OP_RAMPD + OP_EIND + OP_SREG + : "memory", "r31"); +} + +__attribute__((used,naked,noinline,noclone)) +static void host_store2 (void) +{ + __asm __volatile__ + ("nop" + CR ".global do_stores_after" + CR ".type do_stores_after,@function" + CR "do_stores_after:" + /* Store core regs after ISR */ + ST_REGS_LO (mem2) + ST_REGS_HI (mem2) + ST_RAMPX (mem2) + ST_RAMPY (mem2) + ST_RAMPZ (mem2) + ST_RAMPD (mem2) + ST_EIND (mem2) + ST_SREG (mem2) + /* Undo funny values */ + CR "clr __zero_reg__" + OO_RAMPX + OO_RAMPY + OO_RAMPZ + CR "ret" + : /* No outputs */ + : [mem2] "i" (®info2), [first] "n" (FIRST_REG) + OP_RAMPX + OP_RAMPY + OP_RAMPZ + OP_RAMPD + OP_EIND + OP_SREG + : "memory"); +} + +#define MK_CALL_ISR(vecno) \ + __asm __volatile__ \ + (/* Funny values to some SFRs */ \ + /* Must set I-flag due to RETI of ISR */ \ + /* Store core regs before ISR */ \ + CR "%~call do_stores_before" \ + /* Execute ISR */ \ + CR "%~call __vectors + %[vect]" \ + /* Store core regs after ISR */ \ + /* Undo funny values */ \ + CR "%~call do_stores_after" \ + : /* No outputs */ \ + : [vect] "i" (VEC_SIZE * (vecno)) \ + , "i" (host_store1) \ + , "i" (host_store2) \ + : "memory", "r31") + + +#define MK_RUN_ISR(N, IGMSK) \ + \ +__attribute__((noinline,noclone)) \ +void run_isr_ ## N (void) \ +{ \ + clear_reginfo(); \ + MK_CALL_ISR (N); \ + compare_reginfo (IGMSK); \ +} + +#endif /* ISR_TEST_H */ + Index: testsuite/gcc.target/avr/torture/isr-01-simple.c =================================================================== --- testsuite/gcc.target/avr/torture/isr-01-simple.c (nonexistent) +++ testsuite/gcc.target/avr/torture/isr-01-simple.c (revision 249998) @@ -0,0 +1,98 @@ +/* { dg-do run } */ +/* { dg-options "-std=c99" } */ + +#include "../isr-test.h" + +int volatile v; + +/**********************************************************************/ + +ISR (1, signal) +{ +} + +MK_RUN_ISR (1, 0) + +void test1 (void) +{ + run_isr_1(); +} + +/**********************************************************************/ + +ISR (2, signal) +{ + v++; +} + +MK_RUN_ISR (2, 0) + +void test2 (void) +{ + v = 0; + run_isr_2(); + if (v != 1) + __builtin_abort(); +} + + +/**********************************************************************/ + +ISR (3, signal) +{ + __asm __volatile__ ("$ lds r27, v" + "$ swap r27" + "$ sts v, r27" + ::: "memory", "r27"); +} + +MK_RUN_ISR (3, 0) + +void test3 (void) +{ + run_isr_3(); + if (v != 0x10) + __builtin_abort(); +} + +/**********************************************************************/ + +ISR (4, signal) +{ + __asm __volatile__ ("sts v,__zero_reg__" ::: "memory"); +} + +MK_RUN_ISR (4, 0) + +void test4 (void) +{ + run_isr_4(); + if (v != 0) + __builtin_abort(); +} + +/**********************************************************************/ + +ISR (5, signal) +{ + __asm __volatile__ ("clt"); +} + +MK_RUN_ISR (5, 0) + +void test5 (void) +{ + run_isr_5(); +} + +/**********************************************************************/ + +int main (void) +{ + test1(); + test2(); + test3(); + test4(); + test5(); + return 0; +} Index: testsuite/gcc.target/avr/torture/isr-02-call.c =================================================================== --- testsuite/gcc.target/avr/torture/isr-02-call.c (nonexistent) +++ testsuite/gcc.target/avr/torture/isr-02-call.c (revision 249998) @@ -0,0 +1,60 @@ +/* { dg-do run } */ +/* { dg-options "-std=c99" } */ + +#include "../isr-test.h" + +int volatile v; + +__attribute__((noinline,noclone)) +void inc_v (void) +{ + v++; +} + +/**********************************************************************/ + +ISR (1, signal) +{ + inc_v(); +} + +MK_RUN_ISR (1, 0) + +void test1 (void) +{ + run_isr_1(); + if (v != 1) + __builtin_abort(); +} + +/**********************************************************************/ + +ISR (2, signal) +{ + if (v == 1) + inc_v(); + else + v += 2; +} + +MK_RUN_ISR (2, 0) + +void test2 (void) +{ + run_isr_2(); + if (v != 2) + __builtin_abort(); + run_isr_2(); + if (v != 4) + __builtin_abort(); +} + + +/**********************************************************************/ + +int main (void) +{ + test1(); + test2(); + return 0; +} Index: testsuite/gcc.target/avr/torture/isr-03-fixed.c =================================================================== --- testsuite/gcc.target/avr/torture/isr-03-fixed.c (nonexistent) +++ testsuite/gcc.target/avr/torture/isr-03-fixed.c (revision 249998) @@ -0,0 +1,146 @@ +/* { dg-do run } */ +/* { dg-options "-std=gnu99 -fno-lto -fno-toplevel-reorder" } */ + +// No LTO for now due to PR lto/68384. + +#ifdef __AVR_TINY__ +unsigned char reg2; +#else +register unsigned char reg2 __asm("r2"); +#endif + +#include "../isr-test.h" + +#define SET_REG(reg,val) \ + do { \ + reg = (val); \ + __asm __volatile__("" : "+r" (reg)); \ + } while (0) \ + +#define GET_REG(reg) \ + ({ \ + __asm __volatile__("" : "+r" (reg)); \ + reg; \ + }) + +/**********************************************************************/ + +ISR (1, signal) +{ + reg2++; +} + +MK_RUN_ISR (1, 1ul << 2) + +void test1 (void) +{ + SET_REG (reg2, 0); + run_isr_1(); + if (GET_REG (reg2) != 1) + __builtin_abort(); +} + +/**********************************************************************/ + +__attribute__((noinline,noclone)) +void inc_r2 (void) +{ + reg2++; +} + +ISR (2, signal) +{ + inc_r2 (); +} + +MK_RUN_ISR (2, 1ul << 2) + +void test2 (void) +{ + run_isr_2(); + if (GET_REG (reg2) != 2) + __builtin_abort(); +} + + +/**********************************************************************/ + +ISR (3, signal) +{ +#ifndef __AVR_TINY__ + register char r4 __asm ("r4"); + __asm __volatile ("inc %0" : "+r" (r4)); + __asm __volatile ("inc r5" ::: "r5"); +#endif +} + +MK_RUN_ISR (3, 0) + +void test3 (void) +{ + run_isr_3(); +} + + +/**********************************************************************/ + +#define CLOBB(reg) \ + do { \ + __asm __volatile__ ("inc " #reg ::: #reg); \ + } while (0) + +ISR (4, signal) +{ + char volatile v; + v = 1; + +#ifndef __AVR_TINY__ + CLOBB (r3); + CLOBB (r4); + CLOBB (r5); + CLOBB (r6); + CLOBB (r7); + CLOBB (r8); + CLOBB (r9); + CLOBB (r10); + CLOBB (r11); + CLOBB (r12); + CLOBB (r13); + CLOBB (r14); + CLOBB (r15); + CLOBB (r16); + CLOBB (r17); +#endif + + CLOBB (r18); + CLOBB (r19); + CLOBB (r20); + CLOBB (r21); + CLOBB (r22); + CLOBB (r23); + CLOBB (r24); + CLOBB (r25); + CLOBB (r26); + CLOBB (r27); + CLOBB (r30); + CLOBB (r31); +} + +MK_RUN_ISR (4, 0) + +void test4 (void) +{ + run_isr_4(); +} + + +/**********************************************************************/ + +int main (void) +{ + test1(); + test2(); + test3(); + test4(); + return 0; +}