From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106166 invoked by alias); 12 Jan 2016 16:28:59 -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 106142 invoked by uid 89); 12 Jan 2016 16:28:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=Okay, NULL_RTX, CONST, null_rtx X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 12 Jan 2016 16:28:57 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7A0B53A8; Tue, 12 Jan 2016 08:28:20 -0800 (PST) Received: from e105689-lin.cambridge.arm.com (e105689-lin.cambridge.arm.com [10.2.207.32]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AEF833F246; Tue, 12 Jan 2016 08:28:54 -0800 (PST) Subject: Re: [4.9][PR69082]Backport "[PATCH][ARM]Tighten the conditions for arm_movw, arm_movt" To: Renlin Li , "gcc-patches@gcc.gnu.org" References: <55D4949E.1010404@arm.com> <56951C40.3080100@foss.arm.com> Cc: Kyrylo Tkachov , Ramana Radhakrishnan From: "Richard Earnshaw (lists)" Message-ID: <569529C5.2090508@arm.com> Date: Tue, 12 Jan 2016 16:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <56951C40.3080100@foss.arm.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2016-01/txt/msg00746.txt.bz2 On 12/01/16 15:31, Renlin Li wrote: > Hi all, > > Here I backport r227129 to branch 4.9 to fix exactly the same issue > reported in PR69082. > It's been already committed on trunk and backportted to branch 5. > > > I have quoted the original message for the explanation. > The patch applies to branch 4.9 without any modifications. > Test case is not added as the one provided in the bugzilla ticket is too > big and complex. > > arm-none-linux-gnueabihf regression tested without any issues. > > Is Okay to backport to branch 4.9? > > Renlin Li > > > gcc/ChangeLog > > 2016-01-08 Renlin Li > > PR target/69082 > Backport from mainline: > 2015-08-24 Renlin Li > > * config/arm/arm-protos.h (arm_valid_symbolic_address_p): Declare. > * config/arm/arm.c (arm_valid_symbolic_address_p): Define. > * config/arm/arm.md (arm_movt): Use arm_valid_symbolic_address_p. > * config/arm/constraints.md ("j"): Add check for high code > > OK. R. > On 19/08/15 15:37, Renlin Li wrote: >> >>> On 19/08/15 12:49, Renlin Li wrote: >>>> Hi all, >>>> >>>> This simple patch will tighten the conditions when matching movw and >>>> arm_movt rtx pattern. >>>> Those two patterns will generate the following assembly: >>>> >>>> movw w1, #:lower16: dummy + addend >>>> movt w1, #:upper16: dummy + addend >>>> >>>> The addend here is optional. However, it should be an 16-bit signed >>>> value with in the range -32768 <= A <= 32768. >>>> >>>> By impose this restriction explicitly, it will prevent LRA/reload code >>>> from generation invalid high/lo_sum code for arm target. >>>> In process_address_1(), if the address is not legitimate, it will >>>> try to >>>> generate high/lo_sum pair to put the address into register. It will >>>> check if the target support those newly generated reload instructions. >>>> By define those two patterns, arm will reject them if conditions is not >>>> meet. >>>> >>>> Otherwise, it might generate movw/movt instructions with addend larger >>>> than 32768, this will cause a GAS error. GAS will produce '''offset out >>>> of range'' error message when the addend for MOVW/MOVT REL >>>> relocation is >>>> too large. >>>> >>>> >>>> arm-none-eabi regression tests Okay, Okay to commit to the trunk and >>>> backport to 5.0? >>>> >>>> Regards, >>>> Renlin >>>> >>>> gcc/ChangeLog: >>>> >>>> 2015-08-19 Renlin Li >>>> >>>> * config/arm/arm-protos.h (arm_valid_symbolic_address_p): >>>> Declare. >>>> * config/arm/arm.c (arm_valid_symbolic_address_p): Define. >>>> * config/arm/arm.md (arm_movt): Use >>>> arm_valid_symbolic_address_p. >>>> * config/arm/constraints.md ("j"): Add check for high code. >> >> Thank you, >> Renlin >> > > > backport.diff > > > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > index cef9eec..ff168bf 100644 > --- a/gcc/config/arm/arm-protos.h > +++ b/gcc/config/arm/arm-protos.h > @@ -319,6 +319,7 @@ extern int vfp3_const_double_for_bits (rtx); > > extern void arm_emit_coreregs_64bit_shift (enum rtx_code, rtx, rtx, rtx, rtx, > rtx); > +extern bool arm_valid_symbolic_address_p (rtx); > extern bool arm_validize_comparison (rtx *, rtx *, rtx *); > #endif /* RTX_CODE */ > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index c2095a3..7cc4d93 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -28664,6 +28664,38 @@ arm_emit_coreregs_64bit_shift (enum rtx_code code, rtx out, rtx in, > #undef BRANCH > } > > +/* Returns true if the pattern is a valid symbolic address, which is either a > + symbol_ref or (symbol_ref + addend). > + > + According to the ARM ELF ABI, the initial addend of REL-type relocations > + processing MOVW and MOVT instructions is formed by interpreting the 16-bit > + literal field of the instruction as a 16-bit signed value in the range > + -32768 <= A < 32768. */ > + > +bool > +arm_valid_symbolic_address_p (rtx addr) > +{ > + rtx xop0, xop1 = NULL_RTX; > + rtx tmp = addr; > + > + if (GET_CODE (tmp) == SYMBOL_REF || GET_CODE (tmp) == LABEL_REF) > + return true; > + > + /* (const (plus: symbol_ref const_int)) */ > + if (GET_CODE (addr) == CONST) > + tmp = XEXP (addr, 0); > + > + if (GET_CODE (tmp) == PLUS) > + { > + xop0 = XEXP (tmp, 0); > + xop1 = XEXP (tmp, 1); > + > + if (GET_CODE (xop0) == SYMBOL_REF && CONST_INT_P (xop1)) > + return IN_RANGE (INTVAL (xop1), -0x8000, 0x7fff); > + } > + > + return false; > +} > > /* Returns true if a valid comparison operation and makes > the operands in a form that is valid. */ > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > index 288bbb9..eefb7fa 100644 > --- a/gcc/config/arm/arm.md > +++ b/gcc/config/arm/arm.md > @@ -5774,7 +5774,7 @@ > [(set (match_operand:SI 0 "nonimmediate_operand" "=r") > (lo_sum:SI (match_operand:SI 1 "nonimmediate_operand" "0") > (match_operand:SI 2 "general_operand" "i")))] > - "arm_arch_thumb2" > + "arm_arch_thumb2 && arm_valid_symbolic_address_p (operands[2])" > "movt%?\t%0, #:upper16:%c2" > [(set_attr "predicable" "yes") > (set_attr "predicable_short_it" "no") > diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md > index 42935a4..f9e11e0 100644 > --- a/gcc/config/arm/constraints.md > +++ b/gcc/config/arm/constraints.md > @@ -67,7 +67,8 @@ > (define_constraint "j" > "A constant suitable for a MOVW instruction. (ARM/Thumb-2)" > (and (match_test "TARGET_32BIT && arm_arch_thumb2") > - (ior (match_code "high") > + (ior (and (match_code "high") > + (match_test "arm_valid_symbolic_address_p (XEXP (op, 0))")) > (and (match_code "const_int") > (match_test "(ival & 0xffff0000) == 0"))))) > >