From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91698 invoked by alias); 23 Jun 2019 19:03:44 -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 91687 invoked by uid 89); 23 Jun 2019 19:03:44 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_NUMSUBJECT,KAM_STOCKGEN,LIKELY_SPAM_BODY,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*RU:sk:host86-, HX-Spam-Relays-External:sk:host86-, H*r:sk:host86-, placing X-HELO: mail-wm1-f65.google.com Received: from mail-wm1-f65.google.com (HELO mail-wm1-f65.google.com) (209.85.128.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 23 Jun 2019 19:03:40 +0000 Received: by mail-wm1-f65.google.com with SMTP id u8so11165089wmm.1 for ; Sun, 23 Jun 2019 12:03:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=/nxO0Hjyvt3wsjTtSVvOeozfLNPjlLgL7C4wjrSIcQM=; b=XO2XK1EAMFBqwg2mvu/mrcMp9Xq9pPUuSzZXDicqLVn9cFDhR/7+1Y4RploI3Hpp+R r4iC6PrHztiz26Z2hDbpWd1/fseOKGrv6ZbHlKunKhbcqL2wvchtwbPzR88jag0JPLwH PWYtjPwloksnLw9PQrVH8tiDwnSXzOeQTrAheGfEhpbv6O9VLuSmNvB98eRt7UNJb7Py KtzKe93zF5IFbg9ufrcsx/7T7tTZRodrg9qyuh+aLVtbI0wXMyKL8LdkBdktgSW4Qe4X hr4xMC0nl7gEniL1sBWdpBbsLxgbD0V/dLiHcjD9GIb8ojKbhs/LvwAX7oWx53y/7tqf 5aAQ== Return-Path: Received: from localhost (host86-180-62-212.range86-180.btcentralplus.com. [86.180.62.212]) by smtp.gmail.com with ESMTPSA id f190sm2572849wmg.13.2019.06.23.12.03.37 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 23 Jun 2019 12:03:37 -0700 (PDT) Date: Sun, 23 Jun 2019 19:03:00 -0000 From: Andrew Burgess To: Claudiu Zissulescu Cc: gcc-patches@gcc.gnu.org, fbedard@synopsys.com, claziss@synopsys.com Subject: Re: [PATCH] [ARC] Fix PR89838 Message-ID: <20190623190336.GO23204@embecosm.com> References: <20190606073211.6992-1-claziss@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190606073211.6992-1-claziss@gmail.com> X-Fortune: Science is to computer science as hydrodynamics is to plumbing. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-06/txt/msg01397.txt.bz2 * Claudiu Zissulescu [2019-06-06 10:32:11 +0300]: > Hi Andrew, > > This is a proposed fix for bugzilla PR89838 issue. It also needs to be backported to gcc9 and, eventually, gcc8 branches. > > Ok to apply? > Claudiu I had a look through the patch and found just one small nit detailed below. Otherwise this looks fine. Thanks, Andrew > > gcc/ > xxxx-xx-xx Claudiu Zissulescu > > * config/arc/arc.c (arc_symbol_binds_local_p): New function. > (arc_legitimize_pic_address): Simplify and cleanup the function. > (SYMBOLIC_CONST): Remove. > (prepare_pic_move): Likewise. > (prepare_move_operands): Handle complex mov cases here. > (arc_legitimize_address_0): Remove call to > arc_legitimize_pic_address. > (arc_legitimize_address): Remove call to > arc_legitimize_tls_address. > * config/arc/arc.md (movqi_insn): Allow Cm3 match. > (movhi_insn): Likewise. > > /gcc/testsuite > xxxx-xx-xx Claudiu Zissulescu > > * gcc.target/arc/pr89838.c: New file. > --- > gcc/config/arc/arc.c | 215 +++++++------------------ > gcc/config/arc/arc.md | 8 +- > gcc/testsuite/gcc.target/arc/pr89838.c | 16 ++ > 3 files changed, 77 insertions(+), 162 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/arc/pr89838.c > > diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c > index 20dfae66370..a5ee5c49a8f 100644 > --- a/gcc/config/arc/arc.c > +++ b/gcc/config/arc/arc.c > @@ -5986,130 +5986,47 @@ arc_legitimize_tls_address (rtx addr, enum tls_model model) > } > } > > -/* Legitimize a pic address reference in ORIG. > - The return value is the legitimated address. > - If OLDX is non-zero, it is the target to assign the address to first. */ > +/* Return true if SYMBOL_REF X binds locally. */ > > -static rtx > -arc_legitimize_pic_address (rtx orig, rtx oldx) > +static bool > +arc_symbol_binds_local_p (const_rtx x) > { > - rtx addr = orig; > - rtx pat = orig; > - rtx base; > + return (SYMBOL_REF_DECL (x) > + ? targetm.binds_local_p (SYMBOL_REF_DECL (x)) > + : SYMBOL_REF_LOCAL_P (x)); > +} > + > +/* Legitimize a pic address reference in ORIG. The return value is > + the legitimated address. */ This comment needs updating I think, it references ORIG, but there is no ORIG in arc_legitimize_pic. > > - if (oldx == orig) > - oldx = NULL; > +static rtx > +arc_legitimize_pic_address (rtx addr) > +{ > + if (!flag_pic) > + return addr; > > - if (GET_CODE (addr) == LABEL_REF) > - ; /* Do nothing. */ > - else if (GET_CODE (addr) == SYMBOL_REF) > + switch (GET_CODE (addr)) > { > - enum tls_model model = SYMBOL_REF_TLS_MODEL (addr); > - if (model != 0) > - return arc_legitimize_tls_address (addr, model); > - else if (!flag_pic) > - return orig; > - else if (CONSTANT_POOL_ADDRESS_P (addr) || SYMBOL_REF_LOCAL_P (addr)) > - return arc_unspec_offset (addr, ARC_UNSPEC_GOTOFFPC); > + case SYMBOL_REF: > + /* TLS symbols are handled in different place. */ > + if (SYMBOL_REF_TLS_MODEL (addr)) > + return addr; > > /* This symbol must be referenced via a load from the Global > Offset Table (@GOTPC). */ > - pat = arc_unspec_offset (addr, ARC_UNSPEC_GOT); > - pat = gen_const_mem (Pmode, pat); > - > - if (oldx == NULL) > - oldx = gen_reg_rtx (Pmode); > - > - emit_move_insn (oldx, pat); > - pat = oldx; > - } > - else > - { > - if (GET_CODE (addr) == CONST) > - { > - addr = XEXP (addr, 0); > - if (GET_CODE (addr) == UNSPEC) > - { > - /* Check that the unspec is one of the ones we generate? */ > - return orig; > - } > - /* fwprop is placing in the REG_EQUIV notes constant pic > - unspecs expressions. Then, loop may use these notes for > - optimizations resulting in complex patterns that are not > - supported by the current implementation. The following > - two if-cases are simplifying the complex patters to > - simpler ones. */ > - else if (GET_CODE (addr) == MINUS) > - { > - rtx op0 = XEXP (addr, 0); > - rtx op1 = XEXP (addr, 1); > - gcc_assert (oldx); > - gcc_assert (GET_CODE (op1) == UNSPEC); > - > - emit_move_insn (oldx, > - gen_rtx_CONST (SImode, > - arc_legitimize_pic_address (op1, > - NULL_RTX))); > - emit_insn (gen_rtx_SET (oldx, gen_rtx_MINUS (SImode, op0, oldx))); > - return oldx; > - > - } > - else if (GET_CODE (addr) != PLUS) > - { > - rtx tmp = XEXP (addr, 0); > - enum rtx_code code = GET_CODE (addr); > - > - /* It only works for UNARY operations. */ > - gcc_assert (UNARY_P (addr)); > - gcc_assert (GET_CODE (tmp) == UNSPEC); > - gcc_assert (oldx); > - > - emit_move_insn > - (oldx, > - gen_rtx_CONST (SImode, > - arc_legitimize_pic_address (tmp, > - NULL_RTX))); > - > - emit_insn (gen_rtx_SET (oldx, > - gen_rtx_fmt_ee (code, SImode, > - oldx, const0_rtx))); > - > - return oldx; > - } > - else > - { > - gcc_assert (GET_CODE (addr) == PLUS); > - if (GET_CODE (XEXP (addr, 0)) == UNSPEC) > - return orig; > - } > - } > - > - if (GET_CODE (addr) == PLUS) > - { > - rtx op0 = XEXP (addr, 0), op1 = XEXP (addr, 1); > - > - base = arc_legitimize_pic_address (op0, oldx); > - pat = arc_legitimize_pic_address (op1, > - base == oldx ? NULL_RTX : oldx); > + if (!arc_symbol_binds_local_p (addr)) > + return gen_const_mem (Pmode, arc_unspec_offset (addr, ARC_UNSPEC_GOT)); > > - if (base == op0 && pat == op1) > - return orig; > + /* Local symb: use @pcl to access it. */ > + /* Fall through. */ > + case LABEL_REF: > + return arc_unspec_offset (addr, ARC_UNSPEC_GOTOFFPC); > > - if (GET_CODE (pat) == CONST_INT) > - pat = plus_constant (Pmode, base, INTVAL (pat)); > - else > - { > - if (GET_CODE (pat) == PLUS && CONSTANT_P (XEXP (pat, 1))) > - { > - base = gen_rtx_PLUS (Pmode, base, XEXP (pat, 0)); > - pat = XEXP (pat, 1); > - } > - pat = gen_rtx_PLUS (Pmode, base, pat); > - } > - } > + default: > + break; > } > > - return pat; > + return addr; > } > > /* Output address constant X to FILE, taking PIC into account. */ > @@ -6271,28 +6188,6 @@ arc_output_pic_addr_const (FILE * file, rtx x, int code) > } > } > > -#define SYMBOLIC_CONST(X) \ > -(GET_CODE (X) == SYMBOL_REF \ > - || GET_CODE (X) == LABEL_REF \ > - || (GET_CODE (X) == CONST && symbolic_reference_mentioned_p (X))) > - > -/* Emit insns to move operands[1] into operands[0]. */ > - > -static void > -prepare_pic_move (rtx *operands, machine_mode) > -{ > - if (GET_CODE (operands[0]) == MEM && SYMBOLIC_CONST (operands[1]) > - && flag_pic) > - operands[1] = force_reg (Pmode, operands[1]); > - else > - { > - rtx temp = (reload_in_progress ? operands[0] > - : flag_pic? gen_reg_rtx (Pmode) : NULL_RTX); > - operands[1] = arc_legitimize_pic_address (operands[1], temp); > - } > -} > - > - > /* The function returning the number of words, at the beginning of an > argument, must be put in registers. The returned value must be > zero for arguments that are passed entirely in registers or that > @@ -9072,30 +8967,38 @@ prepare_move_operands (rtx *operands, machine_mode mode) > } > } > > - if (mode == SImode && SYMBOLIC_CONST (operands[1])) > + if (GET_CODE (operands[1]) == SYMBOL_REF) > { > - prepare_pic_move (operands, SImode); > - > - /* Disable any REG_EQUALs associated with the symref > - otherwise the optimization pass undoes the work done > - here and references the variable directly. */ > + enum tls_model model = SYMBOL_REF_TLS_MODEL (operands[1]); > + if (MEM_P (operands[0]) && flag_pic) > + operands[1] = force_reg (mode, operands[1]); > + else if (model) > + operands[1] = arc_legitimize_tls_address (operands[1], model); > } > > + operands[1] = arc_legitimize_pic_address (operands[1]); > + > + /* Store instructions are limited, they only accept as address an > + immediate, a register or a register plus a small immediate. */ > if (MEM_P (operands[0]) > - && !(reload_in_progress || reload_completed)) > + && !move_dest_operand (operands[0], mode)) > { > - operands[1] = force_reg (mode, operands[1]); > - if (!move_dest_operand (operands[0], mode)) > - { > - rtx addr = copy_to_mode_reg (Pmode, XEXP (operands[0], 0)); > - /* This is like change_address_1 (operands[0], mode, 0, 1) , > - except that we can't use that function because it is static. */ > - rtx pat = change_address (operands[0], mode, addr); > - MEM_COPY_ATTRIBUTES (pat, operands[0]); > - operands[0] = pat; > - } > + rtx tmp0 = copy_to_mode_reg (Pmode, XEXP (operands[0], 0)); > + rtx tmp1 = change_address (operands[0], mode, tmp0); > + MEM_COPY_ATTRIBUTES (tmp1, operands[0]); > + operands[0] = tmp1; > } > > + /* Check if it is constant but it is not legitimized. */ > + if (CONSTANT_P (operands[1]) > + && !arc_legitimate_constant_p (mode, operands[1])) > + operands[1] = force_reg (mode, XEXP (operands[1], 0)); > + else if (MEM_P (operands[0]) > + && ((CONSTANT_P (operands[1]) > + && !satisfies_constraint_Cm3 (operands[1])) > + || MEM_P (operands[1]))) > + operands[1] = force_reg (mode, operands[1]); > + > return false; > } > > @@ -9566,11 +9469,10 @@ arc_legitimize_address_0 (rtx x, rtx oldx ATTRIBUTE_UNUSED, > { > rtx addr, inner; > > - if (flag_pic && SYMBOLIC_CONST (x)) > - (x) = arc_legitimize_pic_address (x, 0); > addr = x; > if (GET_CODE (addr) == CONST) > addr = XEXP (addr, 0); > + > if (GET_CODE (addr) == PLUS > && CONST_INT_P (XEXP (addr, 1)) > && ((GET_CODE (XEXP (addr, 0)) == SYMBOL_REF > @@ -9601,13 +9503,6 @@ arc_legitimize_address_0 (rtx x, rtx oldx ATTRIBUTE_UNUSED, > static rtx > arc_legitimize_address (rtx orig_x, rtx oldx, machine_mode mode) > { > - if (GET_CODE (orig_x) == SYMBOL_REF) > - { > - enum tls_model model = SYMBOL_REF_TLS_MODEL (orig_x); > - if (model != 0) > - return arc_legitimize_tls_address (orig_x, model); > - } > - > rtx new_x = arc_legitimize_address_0 (orig_x, oldx, mode); > > if (new_x) > diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md > index 082b5bba6ec..ed29aa3d06e 100644 > --- a/gcc/config/arc/arc.md > +++ b/gcc/config/arc/arc.md > @@ -672,7 +672,9 @@ core_3, archs4x, archs4xd, archs4xd_slow" > [(set (match_operand:QI 0 "move_dest_operand" "=Rcq,Rcq#q, w,Rcq#q, h, w, w,???w,h, w,Rcq, S,!*x, r,r, Ucm,m,???m, m,Usc") > (match_operand:QI 1 "move_src_operand" " cL, cP,Rcq#q, P,hCm1,cL, I,?Rac,i,?i, T,Rcq,Usd,Ucm,m,?Rac,c,?Rac,Cm3,i"))] > "register_operand (operands[0], QImode) > - || register_operand (operands[1], QImode)" > + || register_operand (operands[1], QImode) > + || (satisfies_constraint_Cm3 (operands[1]) > + && memory_operand (operands[0], QImode))" > "@ > mov%? %0,%1%& > mov%? %0,%1%& > @@ -714,7 +716,9 @@ core_3, archs4x, archs4xd, archs4xd_slow" > /* Don't use a LIMM that we could load with a single insn - we loose > delay-slot filling opportunities. */ > && !satisfies_constraint_I (operands[1]) > - && satisfies_constraint_Usc (operands[0]))" > + && satisfies_constraint_Usc (operands[0])) > + || (satisfies_constraint_Cm3 (operands[1]) > + && memory_operand (operands[0], HImode))" > "@ > mov%? %0,%1%& > mov%? %0,%1%& > diff --git a/gcc/testsuite/gcc.target/arc/pr89838.c b/gcc/testsuite/gcc.target/arc/pr89838.c > new file mode 100644 > index 00000000000..559434ac87e > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arc/pr89838.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target tls } */ > +/* { dg-options "-O2" } */ > + > +extern void foo (void); > +extern void bar (void *); > + > +struct { > + int __attribute__(()) a; > + int __attribute__(()) b; > +} __thread c __attribute__((tls_model("initial-exec"))); > + > +void foo (void) > +{ > + bar (&c.b); > +} > -- > 2.21.0 >