From patchwork Thu Nov 29 14:42:11 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Gretton-Dann X-Patchwork-Id: 13291 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 250A923FCC for ; Thu, 29 Nov 2012 14:42:16 +0000 (UTC) Received: from mail-ie0-f180.google.com (mail-ie0-f180.google.com [209.85.223.180]) by fiordland.canonical.com (Postfix) with ESMTP id 894D8A18EF8 for ; Thu, 29 Nov 2012 14:42:15 +0000 (UTC) Received: by mail-ie0-f180.google.com with SMTP id c10so10118841ieb.11 for ; Thu, 29 Nov 2012 06:42:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf :mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-gm-message-state; bh=TyxL4mM5/h9RdiwMMNA1iQG/9nwI3NzEr3Ie2uT500c=; b=Kku9flY/shZHsxe8a+ac0bkz+mI1HYcddmehNVy17FONc2huzIz2gV/PNOfqN5UHNn OlJZ3oMo8xP4snwkUKAJvjDWE7wrFQSm7D07sd/n6EgDIGKdk+nKqqtgKhbOX2SxRVNw B1yDGdSqIOS66YHcgZzgHJmYpCKA9VllsKQts/s+FKJua/BkSQAB1BtvtahhVshbWJ6G vd60k3MejEw/SPrrhsWo4Wn7v+32UugM5OONNuqar6OjV8KY2Cv1hvfZeN/l/OD6m1V3 P1W70eHOhE/XlKtc5fEPnJoK18ozTVurq5ul8pkvw0OnbOhH5Cf3ifmOQWZSn2vLx09Z 5iBQ== Received: by 10.50.186.199 with SMTP id fm7mr22578938igc.62.1354200134985; Thu, 29 Nov 2012 06:42:14 -0800 (PST) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.50.67.148 with SMTP id n20csp857301igt; Thu, 29 Nov 2012 06:42:13 -0800 (PST) Received: by 10.152.145.169 with SMTP id sv9mr22207714lab.2.1354200132550; Thu, 29 Nov 2012 06:42:12 -0800 (PST) Received: from mail-lb0-f175.google.com (mail-lb0-f175.google.com [209.85.217.175]) by mx.google.com with ESMTPS id nm5si1310180lab.17.2012.11.29.06.42.11 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 29 Nov 2012 06:42:12 -0800 (PST) Received-SPF: neutral (google.com: 209.85.217.175 is neither permitted nor denied by best guess record for domain of matthew.gretton-dann@linaro.org) client-ip=209.85.217.175; Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.217.175 is neither permitted nor denied by best guess record for domain of matthew.gretton-dann@linaro.org) smtp.mail=matthew.gretton-dann@linaro.org Received: by mail-lb0-f175.google.com with SMTP id gg13so8896143lbb.34 for ; Thu, 29 Nov 2012 06:42:11 -0800 (PST) MIME-Version: 1.0 Received: by 10.152.103.18 with SMTP id fs18mr21818354lab.32.1354200131556; Thu, 29 Nov 2012 06:42:11 -0800 (PST) Received: by 10.114.17.10 with HTTP; Thu, 29 Nov 2012 06:42:11 -0800 (PST) In-Reply-To: References: <2179328.yVVT7qyyS9@e103209-lin> Date: Thu, 29 Nov 2012 14:42:11 +0000 Message-ID: Subject: Re: [RFA/ARM] Fix PR54974: Thumb literal pools don't handle PC rounding From: Matthew Gretton-Dann To: ramrad01@arm.com Cc: "gcc-patches@gcc.gnu.org" , Richard Earnshaw , doko@canonical.com, Patch Tracking X-Gm-Message-State: ALoCoQndV27m9BCWwFh3lYjVSNCCb/oRHs3Y9J7YX2vnfNISFDDUUa6FBW/J/4Pf/dZRkIQcbuBy On 24 November 2012 00:27, Ramana Radhakrishnan wrote: > On Wed, Nov 21, 2012 at 7:59 PM, Matthew Gretton-Dann > wrote: [snip] >> The fix is to decrease the pool_range of all insns by 2 when generating >> Thumb code. There is no need to change neg_pool_range values as rounding >> down here will reduce the distance of the literal pool. > > A comment about this fact around thumb2_pool_range would be appropriate. > [snip] >> >> Tested arm-none-linux-gnueabi cross, and with the testcase attached to the >> PR. No added testcase in the patch as this code is sensitive to other code >> generation and so it is not easy to generate a testcase which will reliably >> test this condition. >> >> OK for trunk, 4.7, and 4.6? > > > Ok for trunk today - please wait a few days before backporting into > 4.6 and 4.7 to see what the fallout is like . Watch out for any > fallout with the auto-testers. The attached is what was actually committed as revision 193930 (original patch + requested comment). Thanks, Matt --- Matthew Gretton-Dann Linaro Toolchain Working Group matthew.gretton-dann@linaro.org Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 193929) +++ gcc/ChangeLog (revision 193930) @@ -1,3 +1,33 @@ +2012-11-29 Matthew Gretton-Dann + + PR target/54974 + * config/arm/arm.md (thumb2_pool_range, pool_range): Add comment on + Thumb pool ranges. + (thumb1_extendhisi2): Reduce Thumb pool range. + (arm_movdi): Likewise. + (thumb1_movdi_insn): Likewise. + (thumb1_movsi_insn): Likewise. + (pic_load_addr_unified): Likewise. + (pic_load_addr_32bit): Likewise. + (pic_load_addr_thumb1): Likewise. + (thumb1_movhf): Likewise. + (arm_movsf_soft_insn): Likewise. + (thumb1_movsf_soft_insn): Likewise. + (movdf_soft_insn): Likewise. + (thumb1_movdf_soft_insn): Likewise. + * config/arm/neon.md (*neon_mov): Likewise. + (*neon_mov): Likwise. + * config/arm/thumb2.md: (*thumb2_movsi_insn): Likewise. + (*thumb2_movhi_insn): Likewise. + (*thumb2_extendqisi_v6): Likewise. + (*thumb2_zero_extendqisi_v6): Likewise. + (*thumb2_zero_extendqisi2_v6): Likewise. + * config/arm/vfp.md: (*thumb2_movsi_vfp): Likewise. + (*movdi_vfp): Likewise. + (*movdi_vfp_cortexa8): Likewise. + (*thumb2_movsf_vfp): Likewise. + (*thumb2_movdf_vfp): Likewise. + 2012-11-29 Kai Tietz PR target/55171 Index: gcc/config/arm/thumb2.md =================================================================== --- gcc/config/arm/thumb2.md (revision 193929) +++ gcc/config/arm/thumb2.md (revision 193930) @@ -182,7 +182,7 @@ str%?\\t%1, %0" [(set_attr "type" "*,*,*,*,load1,load1,store1,store1") (set_attr "predicable" "yes") - (set_attr "pool_range" "*,*,*,*,1020,4096,*,*") + (set_attr "pool_range" "*,*,*,*,1018,4094,*,*") (set_attr "neg_pool_range" "*,*,*,*,0,0,*,*")] ) @@ -217,7 +217,7 @@ ldr%(h%)\\t%0, %1\\t%@ movhi" [(set_attr "type" "*,*,store1,load1") (set_attr "predicable" "yes") - (set_attr "pool_range" "*,*,*,4096") + (set_attr "pool_range" "*,*,*,4094") (set_attr "neg_pool_range" "*,*,*,250")] ) @@ -570,7 +570,7 @@ ldr%(sb%)\\t%0, %1" [(set_attr "type" "alu_shift,load_byte") (set_attr "predicable" "yes") - (set_attr "pool_range" "*,4096") + (set_attr "pool_range" "*,4094") (set_attr "neg_pool_range" "*,250")] ) @@ -583,7 +583,7 @@ ldr%(h%)\\t%0, %1" [(set_attr "type" "alu_shift,load_byte") (set_attr "predicable" "yes") - (set_attr "pool_range" "*,4096") + (set_attr "pool_range" "*,4094") (set_attr "neg_pool_range" "*,250")] ) @@ -596,7 +596,7 @@ ldr%(b%)\\t%0, %1\\t%@ zero_extendqisi2" [(set_attr "type" "alu_shift,load_byte") (set_attr "predicable" "yes") - (set_attr "pool_range" "*,4096") + (set_attr "pool_range" "*,4094") (set_attr "neg_pool_range" "*,250")] ) Index: gcc/config/arm/vfp.md =================================================================== --- gcc/config/arm/vfp.md (revision 193929) +++ gcc/config/arm/vfp.md (revision 193930) @@ -123,7 +123,7 @@ (set_attr "type" "*,*,*,*,load1,load1,store1,store1,r_2_f,f_2_r,fcpys,f_loads,f_stores") (set_attr "neon_type" "*,*,*,*,*,*,*,*,neon_mcr,neon_mrc,neon_vmov,*,*") (set_attr "insn" "mov,mov,mvn,mov,*,*,*,*,*,*,*,*,*") - (set_attr "pool_range" "*,*,*,*,1020,4096,*,*,*,*,*,1020,*") + (set_attr "pool_range" "*,*,*,*,1018,4094,*,*,*,*,*,1018,*") (set_attr "neg_pool_range" "*,*,*,*, 0, 0,*,*,*,*,*,1008,*")] ) @@ -176,7 +176,8 @@ (const_int 8) (const_int 4))] (const_int 4))) - (set_attr "pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*") + (set_attr "arm_pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*") + (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*") (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*") (set_attr "arch" "t2,any,any,any,a,t2,any,any,any,any,any,any")] ) @@ -224,7 +225,8 @@ * 4")] (const_int 4))) (set_attr "predicable" "yes") - (set_attr "pool_range" "*,*,*,*,1020,4096,*,*,*,*,1020,*") + (set_attr "arm_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*") + (set_attr "thumb2_pool_range" "*,*,*,*,1018,4094,*,*,*,*,1018,*") (set_attr "neg_pool_range" "*,*,*,*,1004,0,*,*,*,*,1004,*") (set (attr "ce_count") (symbol_ref "get_attr_length (insn) / 4")) @@ -413,7 +415,7 @@ "r_2_f,f_2_r,fconsts,f_loads,f_stores,load1,store1,fcpys,*") (set_attr "neon_type" "neon_mcr,neon_mrc,*,*,*,*,*,neon_vmov,*") (set_attr "insn" "*,*,*,*,*,*,*,*,mov") - (set_attr "pool_range" "*,*,*,1020,*,4092,*,*,*") + (set_attr "pool_range" "*,*,*,1018,*,4090,*,*,*") (set_attr "neg_pool_range" "*,*,*,1008,*,0,*,*,*")] ) @@ -509,7 +511,7 @@ (const_int 8) (const_int 4))] (const_int 4))) - (set_attr "pool_range" "*,*,*,1020,*,4096,*,*,*") + (set_attr "pool_range" "*,*,*,1018,*,4094,*,*,*") (set_attr "neg_pool_range" "*,*,*,1008,*,0,*,*,*")] ) Index: gcc/config/arm/neon.md =================================================================== --- gcc/config/arm/neon.md (revision 193929) +++ gcc/config/arm/neon.md (revision 193930) @@ -196,7 +196,8 @@ (set_attr "type" "*,f_stored,*,f_loadd,*,*,alu,load2,store2") (set_attr "insn" "*,*,*,*,*,*,mov,*,*") (set_attr "length" "4,4,4,4,4,4,8,8,8") - (set_attr "pool_range" "*,*,*,1020,*,*,*,1020,*") + (set_attr "arm_pool_range" "*,*,*,1020,*,*,*,1020,*") + (set_attr "thumb2_pool_range" "*,*,*,1018,*,*,*,1018,*") (set_attr "neg_pool_range" "*,*,*,1004,*,*,*,1004,*")]) (define_insn "*neon_mov" @@ -241,7 +242,8 @@ (set_attr "type" "*,*,*,*,*,*,alu,load4,store4") (set_attr "insn" "*,*,*,*,*,*,mov,*,*") (set_attr "length" "4,8,4,8,8,8,16,8,16") - (set_attr "pool_range" "*,*,*,1020,*,*,*,1020,*") + (set_attr "arm_pool_range" "*,*,*,1020,*,*,*,1020,*") + (set_attr "thumb2_pool_range" "*,*,*,1018,*,*,*,1018,*") (set_attr "neg_pool_range" "*,*,*,996,*,*,*,996,*")]) (define_expand "movti" Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 193929) +++ gcc/config/arm/arm.md (revision 193930) @@ -297,6 +297,9 @@ ; POOL_RANGE is how far away from a constant pool entry that this insn ; can be placed. If the distance is zero, then this insn will never ; reference the pool. +; Note that for Thumb constant pools the PC value is rounded down to the +; nearest multiple of four. Therefore, THUMB2_POOL_RANGE (and POOL_RANGE for +; Thumb insns) should be set to - 2. ; NEG_POOL_RANGE is nonzero for insns that can reference a constant pool entry ; before its address. It is set to - (8 + ). (define_attr "arm_pool_range" "" (const_int 0)) @@ -4966,7 +4969,7 @@ (const_int 2) (const_int 4)) (const_int 4)]) (set_attr "type" "alu_shift,load_byte") - (set_attr "pool_range" "*,1020")] + (set_attr "pool_range" "*,1018")] ) ;; This pattern will only be used when ldsh is not available @@ -5373,7 +5376,7 @@ (set_attr "type" "*,*,*,load2,store2") (set_attr "arm_pool_range" "*,*,*,1020,*") (set_attr "arm_neg_pool_range" "*,*,*,1004,*") - (set_attr "thumb2_pool_range" "*,*,*,4096,*") + (set_attr "thumb2_pool_range" "*,*,*,4094,*") (set_attr "thumb2_neg_pool_range" "*,*,*,0,*")] ) @@ -5512,7 +5515,7 @@ [(set_attr "length" "4,4,6,2,2,6,4,4") (set_attr "type" "*,*,*,load2,store2,load2,store2,*") (set_attr "insn" "*,mov,*,*,*,*,*,mov") - (set_attr "pool_range" "*,*,*,*,*,1020,*,*")] + (set_attr "pool_range" "*,*,*,*,*,1018,*,*")] ) (define_expand "movsi" @@ -5682,7 +5685,7 @@ mov\\t%0, %1" [(set_attr "length" "2,2,4,4,2,2,2,2,2") (set_attr "type" "*,*,*,*,load1,store1,load1,store1,*") - (set_attr "pool_range" "*,*,*,*,*,*,1020,*,*") + (set_attr "pool_range" "*,*,*,*,*,*,1018,*,*") (set_attr "conds" "set,clob,*,*,nocond,nocond,nocond,nocond,nocond")]) (define_split @@ -5790,7 +5793,7 @@ (match_dup 2)] UNSPEC_PIC_BASE))] "operands[3] = TARGET_THUMB ? GEN_INT (4) : GEN_INT (8);" [(set_attr "type" "load1,load1,load1") - (set_attr "pool_range" "4096,4096,1024") + (set_attr "pool_range" "4096,4094,1022") (set_attr "neg_pool_range" "4084,0,0") (set_attr "arch" "a,t2,t1") (set_attr "length" "8,6,4")] @@ -5806,7 +5809,10 @@ "TARGET_32BIT && flag_pic" "ldr%?\\t%0, %1" [(set_attr "type" "load1") - (set_attr "pool_range" "4096") + (set (attr "pool_range") + (if_then_else (eq_attr "is_thumb" "no") + (const_int 4096) + (const_int 4094))) (set (attr "neg_pool_range") (if_then_else (eq_attr "is_thumb" "no") (const_int 4084) @@ -5819,7 +5825,7 @@ "TARGET_THUMB1 && flag_pic" "ldr\\t%0, %1" [(set_attr "type" "load1") - (set (attr "pool_range") (const_int 1024))] + (set (attr "pool_range") (const_int 1018))] ) (define_insn "pic_add_dot_plus_four" @@ -6614,7 +6620,7 @@ [(set_attr "length" "2") (set_attr "type" "*,load1,store1,*,*") (set_attr "insn" "mov,*,*,mov,mov") - (set_attr "pool_range" "*,1020,*,*,*") + (set_attr "pool_range" "*,1018,*,*,*") (set_attr "conds" "clob,nocond,nocond,nocond,nocond")]) (define_expand "movsf" @@ -6669,7 +6675,8 @@ [(set_attr "predicable" "yes") (set_attr "type" "*,load1,store1") (set_attr "insn" "mov,*,*") - (set_attr "pool_range" "*,4096,*") + (set_attr "arm_pool_range" "*,4096,*") + (set_attr "thumb2_pool_range" "*,4094,*") (set_attr "arm_neg_pool_range" "*,4084,*") (set_attr "thumb2_neg_pool_range" "*,0,*")] ) @@ -6691,7 +6698,7 @@ mov\\t%0, %1" [(set_attr "length" "2") (set_attr "type" "*,load1,store1,load1,store1,*,*") - (set_attr "pool_range" "*,*,*,1020,*,*,*") + (set_attr "pool_range" "*,*,*,1018,*,*,*") (set_attr "insn" "*,*,*,*,*,mov,mov") (set_attr "conds" "clob,nocond,nocond,nocond,nocond,nocond,nocond")] ) @@ -6780,7 +6787,8 @@ " [(set_attr "length" "8,12,16,8,8") (set_attr "type" "*,*,*,load2,store2") - (set_attr "pool_range" "*,*,*,1020,*") + (set_attr "arm_pool_range" "*,*,*,1020,*") + (set_attr "thumb2_pool_range" "*,*,*,1018,*") (set_attr "arm_neg_pool_range" "*,*,*,1004,*") (set_attr "thumb2_neg_pool_range" "*,*,*,0,*")] ) @@ -6824,7 +6832,7 @@ [(set_attr "length" "4,2,2,6,4,4") (set_attr "type" "*,load2,store2,load2,store2,*") (set_attr "insn" "*,*,*,*,*,mov") - (set_attr "pool_range" "*,*,*,1020,*,*")] + (set_attr "pool_range" "*,*,*,1018,*,*")] )