diff mbox

[i386] : Extend TARGET_READ_MODIFY{,_WRITE} peepholes to all integer modes

Message ID CAFULd4bqYNLd0P4Vb7Oi3F0Y2ieDNG0peGqeD9kww7s6jkSCMw@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak April 28, 2016, 7:16 p.m. UTC
Hello!

Attached patch extends TARGET_READ_MODIFY{,_WRITE} peepholes to handle
all integer modes, while also taking care not to introduce additional
QImode register stalls.

While looking at the insn enable condition, I noticed that we don't
use "probe_stack" pattern any more, as the stack check loop is now
implemented in a different way.

2016-04-28  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.md (peephole2s for operations with memory inputs):
    Use SWI mode iterator.
    (peephole2s for operations with memory outputs): Ditto.
    Do not check for stack checking probe.

    (probe_stack): Remove expander.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.

Comments

Eric Botcazou April 29, 2016, 7:47 a.m. UTC | #1
> While looking at the insn enable condition, I noticed that we don't
> use "probe_stack" pattern any more, as the stack check loop is now
> implemented in a different way.

Yes, we do, probe_stack is a standard pattern called by the middle-end.

> 2016-04-28  Uros Bizjak  <ubizjak@gmail.com>
> 
>     * config/i386/i386.md (peephole2s for operations with memory inputs):
>     Use SWI mode iterator.
>     (peephole2s for operations with memory outputs): Ditto.
>     Do not check for stack checking probe.
> 
>     (probe_stack): Remove expander.
> 
> Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

How did you test it exactly?

                === acats tests ===
FAIL:   c52103x
FAIL:   c52104x
Uros Bizjak April 29, 2016, 7:50 a.m. UTC | #2
On Fri, Apr 29, 2016 at 9:47 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> While looking at the insn enable condition, I noticed that we don't
>> use "probe_stack" pattern any more, as the stack check loop is now
>> implemented in a different way.
>
> Yes, we do, probe_stack is a standard pattern called by the middle-end.
>
>> 2016-04-28  Uros Bizjak  <ubizjak@gmail.com>
>>
>>     * config/i386/i386.md (peephole2s for operations with memory inputs):
>>     Use SWI mode iterator.
>>     (peephole2s for operations with memory outputs): Ditto.
>>     Do not check for stack checking probe.
>>
>>     (probe_stack): Remove expander.
>>
>> Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
>
> How did you test it exactly?
>
>                 === acats tests ===
> FAIL:   c52103x
> FAIL:   c52104x

Apparently without ada...

We can put it back, but prehaps implemented as unspec, so it won't
interfere with peepholes?

Uros.
Eric Botcazou April 29, 2016, 7:58 a.m. UTC | #3
> We can put it back, but prehaps implemented as unspec, so it won't
> interfere with peepholes?

No strong opinion, as long as the final assembly is the same as before.
diff mbox

Patch

Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 235582)
+++ config/i386/i386.md	(working copy)
@@ -17552,20 +17552,6 @@ 
   DONE;
 })
 
-;; Use IOR for stack probes, this is shorter.
-(define_expand "probe_stack"
-  [(match_operand 0 "memory_operand")]
-  ""
-{
-  rtx (*gen_ior3) (rtx, rtx, rtx);
-
-  gen_ior3 = (GET_MODE (operands[0]) == DImode
-	      ? gen_iordi3 : gen_iorsi3);
-
-  emit_insn (gen_ior3 (operands[0], operands[0], const0_rtx));
-  DONE;
-})
-
 (define_insn "adjust_stack_and_probe<mode>"
   [(set (match_operand:P 0 "register_operand" "=r")
 	(unspec_volatile:P [(match_operand:P 1 "register_operand" "0")]
@@ -17894,11 +17880,11 @@ 
 
 ;; Don't do logical operations with memory inputs.
 (define_peephole2
-  [(match_scratch:SI 2 "r")
-   (parallel [(set (match_operand:SI 0 "register_operand")
-                   (match_operator:SI 3 "arith_or_logical_operator"
+  [(match_scratch:SWI 2 "<r>")
+   (parallel [(set (match_operand:SWI 0 "register_operand")
+		   (match_operator:SWI 3 "arith_or_logical_operator"
                      [(match_dup 0)
-                      (match_operand:SI 1 "memory_operand")]))
+		      (match_operand:SWI 1 "memory_operand")]))
               (clobber (reg:CC FLAGS_REG))])]
   "!(TARGET_READ_MODIFY || optimize_insn_for_size_p ())"
   [(set (match_dup 2) (match_dup 1))
@@ -17907,10 +17893,10 @@ 
               (clobber (reg:CC FLAGS_REG))])])
 
 (define_peephole2
-  [(match_scratch:SI 2 "r")
-   (parallel [(set (match_operand:SI 0 "register_operand")
-                   (match_operator:SI 3 "arith_or_logical_operator"
-                     [(match_operand:SI 1 "memory_operand")
+  [(match_scratch:SWI 2 "<r>")
+   (parallel [(set (match_operand:SWI 0 "register_operand")
+		   (match_operator:SWI 3 "arith_or_logical_operator"
+		     [(match_operand:SWI 1 "memory_operand")
                       (match_dup 0)]))
               (clobber (reg:CC FLAGS_REG))])]
   "!(TARGET_READ_MODIFY || optimize_insn_for_size_p ())"
@@ -17962,15 +17948,13 @@ 
 ; the same decoder scheduling characteristics as the original.
 
 (define_peephole2
-  [(match_scratch:SI 2 "r")
-   (parallel [(set (match_operand:SI 0 "memory_operand")
-                   (match_operator:SI 3 "arith_or_logical_operator"
+  [(match_scratch:SWI 2 "<r>")
+   (parallel [(set (match_operand:SWI 0 "memory_operand")
+		   (match_operator:SWI 3 "arith_or_logical_operator"
                      [(match_dup 0)
-                      (match_operand:SI 1 "nonmemory_operand")]))
+		      (match_operand:SWI 1 "<nonmemory_operand>")]))
               (clobber (reg:CC FLAGS_REG))])]
-  "!(TARGET_READ_MODIFY_WRITE || optimize_insn_for_size_p ())
-   /* Do not split stack checking probes.  */
-   && GET_CODE (operands[3]) != IOR && operands[1] != const0_rtx"
+  "!(TARGET_READ_MODIFY_WRITE || optimize_insn_for_size_p ())"
   [(set (match_dup 2) (match_dup 0))
    (parallel [(set (match_dup 2)
                    (match_op_dup 3 [(match_dup 2) (match_dup 1)]))
@@ -17978,15 +17962,13 @@ 
    (set (match_dup 0) (match_dup 2))])
 
 (define_peephole2
-  [(match_scratch:SI 2 "r")
-   (parallel [(set (match_operand:SI 0 "memory_operand")
-                   (match_operator:SI 3 "arith_or_logical_operator"
-                     [(match_operand:SI 1 "nonmemory_operand")
+  [(match_scratch:SWI 2 "<r>")
+   (parallel [(set (match_operand:SWI 0 "memory_operand")
+		   (match_operator:SWI 3 "arith_or_logical_operator"
+		     [(match_operand:SWI 1 "<nonmemory_operand>")
                       (match_dup 0)]))
               (clobber (reg:CC FLAGS_REG))])]
-  "!(TARGET_READ_MODIFY_WRITE || optimize_insn_for_size_p ())
-   /* Do not split stack checking probes.  */
-   && GET_CODE (operands[3]) != IOR && operands[1] != const0_rtx"
+  "!(TARGET_READ_MODIFY_WRITE || optimize_insn_for_size_p ())"
   [(set (match_dup 2) (match_dup 0))
    (parallel [(set (match_dup 2)
                    (match_op_dup 3 [(match_dup 1) (match_dup 2)]))