| Message ID | 558C3576.3070108@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 25, 2015 at 6:08 PM, Michael Collison <michael.collison@linaro.org> wrote: > > This patch is designed to optimize constructs such as: > > #define min(x, y) ((x) <= (y)) ? (x) : (y) > > unsignedint foo (unsignedint i, unsignedint x ,unsignedint y) > { > return i < (min (x, y)); > } > > int bar (int i,int x,int y) > { > return i < (min (x, y)); > } > > Patch was tested on arm-linux-gnueabi, arm-linux-gnueabihf, > armeb-linux-gnueabihf. Okay for trunk? Sorry about the slow review and I wanted someone else to look at it given I had a hand in writing this patch up. Please add a testcase. > > > 2015-06-24 Michael Collison <michael.collison@linaro.org > > 2012-05-01 Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org> Please fix the Changelog formatting here. > > * gcc/config/arm/arm.md (*arm_smin_cmp): New pattern. > (*arm_umin_cmp): Likewise. > > diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md > index 1ac8af0..994c95f 100644 > --- a/gcc/config/arm/arm.md > +++ b/gcc/config/arm/arm.md > @@ -3455,6 +3455,28 @@ > (set_attr "type" "multiple,multiple")] > ) > > +;; t = (s/u)min (x, y) > +;; cc = cmp (t, z) > +;; is the same as > +;; cmp x, z > +;; cmpge(u) y, z > + > +(define_insn_and_split "*arm_smin_cmp" > + [(set (reg:CC CC_REGNUM) > + (compare:CC > + (smin:SI (match_operand:SI 0 "s_register_operand" "r") > + (match_operand:SI 1 "s_register_operand" "r")) > + (match_operand:SI 2 "s_register_operand" "r")))] > + "TARGET_32BIT" > + "#" > + "" > + [(set (reg:CC CC_REGNUM) > + (compare:CC (match_dup 0) (match_dup 2))) > + (cond_exec (ge:CC (reg:CC CC_REGNUM) (const_int 0)) > + (set (reg:CC CC_REGNUM) > + (compare:CC (match_dup 1) (match_dup 2))))] > +) IIUC it's not entirely safe to have cond_execs in the instruction stream prior to reload - I think the consensus was that spilling and filling with cond-exec style instructions could end up with non-cond-exec style spills thus destroying registers in the non cond-exec cases. so, lets just add a reload_completed to be safe here. See https://patches.linaro.org/6469/ for more on this topic. > + > (define_expand "umaxsi3" > [(parallel [ > (set (match_operand:SI 0 "s_register_operand" "") > @@ -3521,6 +3543,22 @@ > (set_attr "type" "store1")] > ) > > +(define_insn_and_split "*arm_umin_cmp" > + [(set (reg:CC CC_REGNUM) > + (compare:CC > + (umin:SI (match_operand:SI 0 "s_register_operand" "r") > + (match_operand:SI 1 "s_register_operand" "r")) > + (match_operand:SI 2 "s_register_operand" "r")))] > + "TARGET_32BIT" > + "#" > + "" > + [(set (reg:CC CC_REGNUM) > + (compare:CC (match_dup 0) (match_dup 2))) > + (cond_exec (geu:CC (reg:CC CC_REGNUM) (const_int 0)) > + (set (reg:CC CC_REGNUM) > + (compare:CC (match_dup 1) (match_dup 2))))] > +) > + Please move this below the other pattern. > (define_insn "*store_minmaxsi" > [(set (match_operand:SI 0 "memory_operand" "=m") > (match_operator:SI 3 "minmax_operator" > > -- > Michael Collison > Linaro Toolchain Working Group > michael.collison@linaro.org > Please repost after testing those changes and then I think this is OK to go in. regards Ramana
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 1ac8af0..994c95f 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -3455,6 +3455,28 @@ (set_attr "type" "multiple,multiple")] ) +;; t = (s/u)min (x, y) +;; cc = cmp (t, z) +;; is the same as +;; cmp x, z +;; cmpge(u) y, z + +(define_insn_and_split "*arm_smin_cmp" + [(set (reg:CC CC_REGNUM) + (compare:CC + (smin:SI (match_operand:SI 0 "s_register_operand" "r") + (match_operand:SI 1 "s_register_operand" "r")) + (match_operand:SI 2 "s_register_operand" "r")))] + "TARGET_32BIT" + "#" + "" + [(set (reg:CC CC_REGNUM) + (compare:CC (match_dup 0) (match_dup 2))) + (cond_exec (ge:CC (reg:CC CC_REGNUM) (const_int 0)) + (set (reg:CC CC_REGNUM) + (compare:CC (match_dup 1) (match_dup 2))))] +) + (define_expand "umaxsi3" [(parallel [ (set (match_operand:SI 0 "s_register_operand" "") @@ -3521,6 +3543,22 @@ (set_attr "type" "store1")] ) +(define_insn_and_split "*arm_umin_cmp" + [(set (reg:CC CC_REGNUM) + (compare:CC + (umin:SI (match_operand:SI 0 "s_register_operand" "r") + (match_operand:SI 1 "s_register_operand" "r")) + (match_operand:SI 2 "s_register_operand" "r")))] + "TARGET_32BIT" + "#" + "" + [(set (reg:CC CC_REGNUM) + (compare:CC (match_dup 0) (match_dup 2))) + (cond_exec (geu:CC (reg:CC CC_REGNUM) (const_int 0)) + (set (reg:CC CC_REGNUM) + (compare:CC (match_dup 1) (match_dup 2))))] +) + (define_insn "*store_minmaxsi" [(set (match_operand:SI 0 "memory_operand" "=m") (match_operator:SI 3 "minmax_operator"
This patch is designed to optimize constructs such as: #define min(x, y) ((x) <= (y)) ? (x) : (y) unsignedint foo (unsignedint i, unsignedint x ,unsignedint y) { return i < (min (x, y)); } int bar (int i,int x,int y) { return i < (min (x, y)); } Patch was tested on arm-linux-gnueabi, arm-linux-gnueabihf, armeb-linux-gnueabihf. Okay for trunk? 2015-06-24 Michael Collison <michael.collison@linaro.org 2012-05-01 Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org> * gcc/config/arm/arm.md (*arm_smin_cmp): New pattern. (*arm_umin_cmp): Likewise.