This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][ARM] Fix low reg issue in Thumb-2 movsi patterns



On 24/07/2019 16:16, Wilco Dijkstra wrote:

The Thumb-2 movsi patterns try to prefer low registers for loads and stores.
However this is done incorrectly by using 2 separate variants with 'l' and 'h'
register classes.  The register allocator will only use low registers, and
as a result we end up with significantly more spills and moves to high
registers.  Fix this by merging the alternatives and use 'l*r' to indicate
preference for low registers.  This saves ~400 instructions from the pr77308
testcase.

Bootstrap & regress OK on arm-none-linux-gnueabihf --with-cpu=cortex-a57

ChangeLog:
2019-07-24  Wilco Dijkstra  <wdijkstr@arm.com>

	* config/arm/thumb2.md (thumb2_movsi_insn): Fix load/store low reg.
	* config/arm/vfp.md (thumb2_movsi_vfp): Likewise.

--
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 78a6ea0b10dab97ed6651ce62e99cfd7a81722ab..c7000d0772a7e5887b6d05be188e8eb38c97217d 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -247,8 +247,8 @@ (define_insn "*thumb2_pop_single"
  ;; regs.  The high register alternatives are not taken into account when
  ;; choosing register preferences in order to reflect their expense.
  (define_insn "*thumb2_movsi_insn"
-  [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,l,r,r,l ,*hk,m,*m")
-	(match_operand:SI 1 "general_operand"	   "rk,I,Py,K,j,mi,*mi,l,*hk"))]
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,l,r,r,l*rk,m")
+	(match_operand:SI 1 "general_operand"	   "rk,I,Py,K,j,mi,l*rk"))]
I think this should be "lk*r", not "l*rk". SP is only going to crop up in rare circumstances, but we are always going to need this pattern if it does and hiding this from register preferencing is pointless. It's not like the compiler is going to start allocating SP in the more general case. 

Same in the other case below, which leads to:
I'd also like to see all these movsi matching patterns merged into a single pattern that just enables the appropriate variants. Having separate implementations for Arm, thumb2, vfp, iwmmx is just making maintenance of this stuff a nightmare. 

R.


    "TARGET_THUMB2 && !TARGET_IWMMXT && !TARGET_HARD_FLOAT
     && (   register_operand (operands[0], SImode)
         || register_operand (operands[1], SImode))"
@@ -262,22 +262,20 @@ (define_insn "*thumb2_movsi_insn"
      case 3: return \"mvn%?\\t%0, #%B1\";
      case 4: return \"movw%?\\t%0, %1\";
      case 5:
-    case 6:
        /* Cannot load it directly, split to load it via MOV / MOVT.  */
        if (!MEM_P (operands[1]) && arm_disable_literal_pool)
  	return \"#\";
        return \"ldr%?\\t%0, %1\";
-    case 7:
-    case 8: return \"str%?\\t%1, %0\";
+    case 6: return \"str%?\\t%1, %0\";
      default: gcc_unreachable ();
      }
  }
-  [(set_attr "type" "mov_reg,mov_imm,mov_imm,mvn_imm,mov_imm,load_4,load_4,store_4,store_4")
-   (set_attr "length" "2,4,2,4,4,4,4,4,4")
+  [(set_attr "type" "mov_reg,mov_imm,mov_imm,mvn_imm,mov_imm,load_4,store_4")
+   (set_attr "length" "2,4,2,4,4,4,4")
     (set_attr "predicable" "yes")
-   (set_attr "predicable_short_it" "yes,no,yes,no,no,no,no,no,no")
-   (set_attr "pool_range" "*,*,*,*,*,1018,4094,*,*")
-   (set_attr "neg_pool_range" "*,*,*,*,*,0,0,*,*")]
+   (set_attr "predicable_short_it" "yes,no,yes,no,no,no,no")
+   (set_attr "pool_range" "*,*,*,*,*,4094,*")
+   (set_attr "neg_pool_range" "*,*,*,*,*,0,*")]
  )
(define_insn "tls_load_dot_plus_four" 
diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md
index e0aaa7b00bb41c046da4531a293e123c94e8b9a4..b59dd6b71d228e042feda3a3a06d81dd01d200da 100644
--- a/gcc/config/arm/vfp.md
+++ b/gcc/config/arm/vfp.md
@@ -258,8 +258,8 @@ (define_insn "*arm_movsi_vfp"
  ;; is chosen with length 2 when the instruction is predicated for
  ;; arm_restrict_it.
  (define_insn "*thumb2_movsi_vfp"
-  [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,l,r,r, l,*hk,m, *m,*t, r,*t,*t,  *Uv")
-	(match_operand:SI 1 "general_operand"	   "rk,I,Py,K,j,mi,*mi,l,*hk, r,*t,*t,*UvTu,*t"))]
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,l,r,r,l*rk,m,*t, r,*t,*t,  *Uv")
+	(match_operand:SI 1 "general_operand"	   "rk,I,Py,K,j,mi,l*rk, r,*t,*t,*UvTu,*t"))]
    "TARGET_THUMB2 && TARGET_HARD_FLOAT
     && (   s_register_operand (operands[0], SImode)
         || s_register_operand (operands[1], SImode))"
@@ -275,32 +275,30 @@ (define_insn "*thumb2_movsi_vfp"
      case 4:
        return \"movw%?\\t%0, %1\";
      case 5:
-    case 6:
        /* Cannot load it directly, split to load it via MOV / MOVT.  */
        if (!MEM_P (operands[1]) && arm_disable_literal_pool)
  	return \"#\";
        return \"ldr%?\\t%0, %1\";
-    case 7:
-    case 8:
+    case 6:
        return \"str%?\\t%1, %0\";
-    case 9:
+    case 7:
        return \"vmov%?\\t%0, %1\\t%@ int\";
-    case 10:
+    case 8:
        return \"vmov%?\\t%0, %1\\t%@ int\";
-    case 11:
+    case 9:
        return \"vmov%?.f32\\t%0, %1\\t%@ int\";
-    case 12: case 13:
+    case 10: case 11:
        return output_move_vfp (operands);
      default:
        gcc_unreachable ();
      }
    "
    [(set_attr "predicable" "yes")
-   (set_attr "predicable_short_it" "yes,no,yes,no,no,no,no,no,no,no,no,no,no,no")
-   (set_attr "type" "mov_reg,mov_reg,mov_reg,mvn_reg,mov_imm,load_4,load_4,store_4,store_4,f_mcr,f_mrc,fmov,f_loads,f_stores")
-   (set_attr "length" "2,4,2,4,4,4,4,4,4,4,4,4,4,4")
-   (set_attr "pool_range"     "*,*,*,*,*,1018,4094,*,*,*,*,*,1018,*")
-   (set_attr "neg_pool_range" "*,*,*,*,*,   0,   0,*,*,*,*,*,1008,*")]
+   (set_attr "predicable_short_it" "yes,no,yes,no,no,no,no,no,no,no,no,no")
+   (set_attr "type" "mov_reg,mov_reg,mov_reg,mvn_reg,mov_imm,load_4,store_4,f_mcr,f_mrc,fmov,f_loads,f_stores")
+   (set_attr "length" "2,4,2,4,4,4,4,4,4,4,4,4")
+   (set_attr "pool_range"     "*,*,*,*,*,4094,*,*,*,*,1018,*")
+   (set_attr "neg_pool_range" "*,*,*,*,*,   0,*,*,*,*,1008,*")]
  )
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]