Patchwork ARM patch: New cbranchqi, cbranchhi patterns for Thumb-1

login
register
mail settings
Submitter Bernd Schmidt
Date July 15, 2010, 1:24 p.m.
Message ID <4C3F0C1F.6020901@codesourcery.com>
Download mbox | patch
Permalink /patch/58983/
State New
Headers show

Comments

Bernd Schmidt - July 15, 2010, 1:24 p.m.
On Thumb-1, comparisons of integer values smaller than a word can be
done by shifting the value to the left.  This patch adds a new macroized
pattern, cbranch<mode>4_insn, which is used for QImode and HImode, and
does exactly that.  Effects:

-       lsl     r2, r1, #24
-       lsr     r2, r2, #24
-       cmp     r2, #0
+       lsl     r1, #24
        bne     .L195

This uses the neat trick of putting a matching constraint on a
match_scratch.

An earlier version (with slightly more stupid code) was regression
tested on qemu/arm-linux with my usual set of three flags.  This version
generates identical code as the tested one for all my testcases.  Ok if
retest passes?


Bernd
* config/arm/arm.md (QHI): New define_mode_iterator.
	(cbranch<mode>4_insn): New pattern.
Richard Earnshaw - July 15, 2010, 2:43 p.m.
On Thu, 2010-07-15 at 15:24 +0200, Bernd Schmidt wrote:
> On Thumb-1, comparisons of integer values smaller than a word can be
> done by shifting the value to the left.  This patch adds a new macroized
> pattern, cbranch<mode>4_insn, which is used for QImode and HImode, and
> does exactly that.  Effects:
> 
> -       lsl     r2, r1, #24
> -       lsr     r2, r2, #24
> -       cmp     r2, #0
> +       lsl     r1, #24
>         bne     .L195
> 
> This uses the neat trick of putting a matching constraint on a
> match_scratch.
> 
> An earlier version (with slightly more stupid code) was regression
> tested on qemu/arm-linux with my usual set of three flags.  This version
> generates identical code as the tested one for all my testcases.  Ok if
> retest passes?
> 
> 
> Bernd

This is going to only set the N and Z flags correctly.  V and C will
have different values.   Having looked at arm_select_cc_mode I can't
quite convince myself that it will be safe for inequality comparisons.
The only relevent test in that that I can see is

  if (GET_MODE (x) == QImode && (op == EQ || op == NE))
    return CC_Zmode;

Which handles your example, but none of the others.

Also note, the same trick should be usable on ARM and Thumb2 if we don't
do that already.

R.

Patch

Index: config/arm/arm.md
===================================================================
--- config/arm/arm.md	(revision 162146)
+++ config/arm/arm.md	(working copy)
@@ -393,6 +393,9 @@  (define_mode_iterator ANY64 [DI DF V8QI 
 ;; The integer modes up to word size
 (define_mode_iterator QHSI [QI HI SI])
 
+;; The integer modes below word size
+(define_mode_iterator QHI [QI HI])
+
 ;;---------------------------------------------------------------------------
 ;; Predicates
 
@@ -6686,6 +6689,43 @@  (define_insn "cbranchsi4_insn"
 		(const_int 8))))]
 )
 
+(define_mode_attr mode_lshift [(QI "24") (HI "16")])
+
+(define_insn "cbranch<mode>4_insn"
+  [(set (pc) (if_then_else
+	      (match_operator 0 "arm_comparison_operator"
+	       [(match_operand:QHI 1 "s_register_operand" "l")
+	        (const_int 0)])
+	      (label_ref (match_operand 2 "" ""))
+	      (pc)))
+   (clobber (match_scratch:QHI 3 "=1"))]
+  "TARGET_THUMB1"
+{
+  output_asm_insn ("lsl\t%1, #<mode_lshift>", operands);
+
+  switch (get_attr_length (insn))
+    {
+    case 4:  return "b%d0\t%l2";
+    case 6:  return "b%D0\t.LCB%=\;b\t%l2\t%@long jump\n.LCB%=:";
+    default: return "b%D0\t.LCB%=\;bl\t%l2\t%@far jump\n.LCB%=:";
+    }
+}
+  [(set (attr "far_jump")
+        (if_then_else
+	    (eq_attr "length" "8")
+	    (const_string "yes")
+            (const_string "no")))
+   (set (attr "length")
+        (if_then_else
+	    (and (ge (minus (match_dup 2) (pc)) (const_int -250))
+	         (le (minus (match_dup 2) (pc)) (const_int 256)))
+	    (const_int 4)
+	    (if_then_else
+	        (and (ge (minus (match_dup 2) (pc)) (const_int -2040))
+		     (le (minus (match_dup 2) (pc)) (const_int 2048)))
+		(const_int 6)
+		(const_int 8))))])
+
 (define_insn "cbranchsi4_scratch"
   [(set (pc) (if_then_else
 	      (match_operator 4 "arm_comparison_operator"