All,

The attached patch fixes PR54974.

In Thumb when calculating the PC value for a literal load the value used is 
the current PC rounded down to the nearest multiple of 4.  The ARM backend 
currently does not take this into account when calculating literal pool 
placement.

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.

The patch attached to the PR is not sufficient as we don't precisely know 
the PC when calculating literal pool ranges and so have to be conservative.

Whilst going through all the code I found the following, possibly related, 
issues that I would like some input from the ARM maintainers on (although 
they have not been touched in this patch):

1) Some Thumb-2 patterns (like thumb2_movhi_insn) have a neg_pool_range of 
250 for ldrh, where my reading of the ARMARM says the range is [-4095, 4095] 
for Thumb-2 (with appropriate rounding).  What is the reason for GCC's 
severe pessimism here?

2) thumb1_zero_extendqisi2 (and other insns) give a Thumb-1 narrow ldrb a 
pool_range of 32.  Surely the pool_range should be 0 (or *) as Thumb-1 
doesn't have a ldrb where the base-register can be PC?

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?

Thanks,

Matt

gcc/ChangeLog:

2012-11-21  Matthew Gretton-Dann  <matthew.gretton-d...@linaro.org>

        PR target/54974
        * config/arm/arm.md (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<mode>): Likewise.
        (*neon_mov<mode>): 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.

--
Matthew Gretton-Dann
Linaro Toolchain Working Group
matthew.gretton-d...@linaro.org
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 7e92b69..b3822d9 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -4952,7 +4952,7 @@ (define_insn "thumb1_extendhisi2"
                                        (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
@@ -5359,7 +5359,7 @@ (define_insn "*arm_movdi"
    (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,*")]
 )
 
@@ -5498,7 +5498,7 @@ (define_insn "*thumb1_movdi_insn"
   [(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"
@@ -5668,7 +5668,7 @@ (define_insn "*thumb1_movsi_insn"
    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 
@@ -5776,7 +5776,7 @@ (define_insn_and_split "pic_load_addr_unified"
                                         (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")]
@@ -5792,7 +5792,10 @@ (define_insn "pic_load_addr_32bit"
   "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)
@@ -5805,7 +5808,7 @@ (define_insn "pic_load_addr_thumb1"
   "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"
@@ -6600,7 +6603,7 @@ (define_insn "*thumb1_movhf"
   [(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"
@@ -6655,7 +6658,8 @@ (define_insn "*arm_movsf_soft_insn"
   [(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,*")]
 )
@@ -6677,7 +6681,7 @@ (define_insn "*thumb1_movsf_insn"
    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")]
 )
@@ -6766,7 +6770,8 @@ (define_insn "*movdf_soft_insn"
   "
   [(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,*")]
 )
@@ -6810,7 +6815,7 @@ (define_insn "*thumb_movdf_insn"
   [(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,*,*")]
 )
 
 
diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index 2103580..8f84795 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -196,7 +196,8 @@ (define_insn "*neon_mov<mode>"
   (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<mode>"
@@ -241,7 +242,8 @@ (define_insn "*neon_mov<mode>"
    (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"
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index a5302f4..533ae18 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -182,7 +182,7 @@ (define_insn "*thumb2_movsi_insn"
    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 @@ (define_insn "*thumb2_movhi_insn"
    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 @@ (define_insn "*thumb2_extendqisi_v6"
    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 @@ (define_insn "*thumb2_zero_extendhisi2_v6"
    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 @@ (define_insn "thumb2_zero_extendqisi2_v6"
    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")]
 )
 
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index d48d4e6..f31d194 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -123,7 +123,7 @@ (define_insn "*thumb2_movsi_vfp"
    (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 @@ (define_insn "*movdi_vfp"
                                  (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 @@ (define_insn "*movdi_vfp_cortexa8"
                                  * 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 @@ (define_insn "*thumb2_movsf_vfp"
      "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 @@ (define_insn "*thumb2_movdf_vfp"
                                 (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,*,*,*")]
 )
 

Reply via email to