diff mbox

[i386] : Fix recently added sibcall insns and peephole2 patterns

Message ID CAFULd4Z5tVoQvNUMv8+w90NVvArSRuCa7t6ni4n=5DmN2poV3w@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak June 18, 2014, 10:24 a.m. UTC 
Hello!

Attached patch fixes recently added sibcall insns and their
corresponding peephole2 patterns:

- There is no need for new memory_nox32_operand. A generic
memory_operand can be used, since new insns and peephole2 patterns
should be disabled for TARGET_X32 entirely.
- Adds missing "m" constraint in insn patterns.
- Macroizes peephole2 patterns
- Adds check that eliminated register is really dead after the call
(maybe an overkill, but some hard-to-debug problems surfaced due to
missing liveness checks in the past)
- Fixes call RTXes in sibcall_pop related patterns (and fixes two
newly introduced warnings in i386.md)

2014-06-18  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.md (*sibcall_memory): Rename from *sibcall_intern.
    Do not use unspec as call operand.  Use memory_operand instead of
    memory_nox32_operand and add "m" operand constraint.  Disable
    pattern for TARGET_X32.
    (*sibcall_pop_memory): Ditto.
    (*sibcall_value_memory): Ditto.
    (*sibcall_value_pop_memory): Ditto.
    (sibcall peepholes): Merge SImode and DImode patterns using
    W mode iterator.  Use memory_operand instead of memory_nox32_operand.
    Disable pattern for TARGET_X32.  Check if eliminated register is
    really dead after call insn.  Generate call RTX without unspec operand.
    (sibcall_value peepholes): Ditto.
    (sibcall_pop peepholes): Fix call insn RTXes.  Use memory_operand
    instead of memory_nox32_operand.  Check if eliminated register is
    really dead after call insn. Generate call RTX without unspec operand.
    (sibcall_value_pop peepholes): Ditto.
    * config/i386/predicates.md (memory_nox32_operand): Remove predicate.

The patch was bootstrapped and regression tested on
x86_64-pc-linux-gnu {,-m32} and was committed to mainline SVN.

Uros.

Comments

Kai Tietz June 18, 2014, 12:24 p.m. UTC | #1 
2014-06-18 12:24 GMT+02:00 Uros Bizjak <ubizjak@gmail.com>:
> Hello!
>
> Attached patch fixes recently added sibcall insns and their
> corresponding peephole2 patterns:
>
> - There is no need for new memory_nox32_operand. A generic
> memory_operand can be used, since new insns and peephole2 patterns
> should be disabled for TARGET_X32 entirely.
> - Adds missing "m" constraint in insn patterns.
> - Macroizes peephole2 patterns
> - Adds check that eliminated register is really dead after the call
> (maybe an overkill, but some hard-to-debug problems surfaced due to
> missing liveness checks in the past)
> - Fixes call RTXes in sibcall_pop related patterns (and fixes two
> newly introduced warnings in i386.md)
>
> 2014-06-18  Uros Bizjak  <ubizjak@gmail.com>
>
>     * config/i386/i386.md (*sibcall_memory): Rename from *sibcall_intern.
>     Do not use unspec as call operand.  Use memory_operand instead of
>     memory_nox32_operand and add "m" operand constraint.  Disable
>     pattern for TARGET_X32.
>     (*sibcall_pop_memory): Ditto.
>     (*sibcall_value_memory): Ditto.
>     (*sibcall_value_pop_memory): Ditto.
>     (sibcall peepholes): Merge SImode and DImode patterns using
>     W mode iterator.  Use memory_operand instead of memory_nox32_operand.
>     Disable pattern for TARGET_X32.  Check if eliminated register is
>     really dead after call insn.  Generate call RTX without unspec operand.
>     (sibcall_value peepholes): Ditto.
>     (sibcall_pop peepholes): Fix call insn RTXes.  Use memory_operand
>     instead of memory_nox32_operand.  Check if eliminated register is
>     really dead after call insn. Generate call RTX without unspec operand.
>     (sibcall_value_pop peepholes): Ditto.
>     * config/i386/predicates.md (memory_nox32_operand): Remove predicate.
>
> The patch was bootstrapped and regression tested on
> x86_64-pc-linux-gnu {,-m32} and was committed to mainline SVN.
>
> Uros.

The following change in predicates.md seems to be a bit premature.
There is still the point about Darwin's PIC issue for unspec-gotpcrel.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61387

  return ANY_QI_REG_P (op);
})

+;; Return true if OP is a memory operands that can be used in sibcalls.
(define_predicate "sibcall_memory_operand"
-  (match_operand 0 "memory_operand")
-{
-  return CONSTANT_P (XEXP (op, 0));
-})
+  (and (match_operand 0 "memory_operand")
+       (match_test "CONSTANT_P (XEXP (op, 0))")))

as we might to pessimize for Darwin UNSPEC_GOTPCREL at that point.
 In general there is still the question why this issue just happens
for Darwin, but not for linux.  For linux that gotpcrel-code path
seems not to be hit at all (at least is that what Ians told).

Kai
Uros Bizjak June 18, 2014, 1:10 p.m. UTC | #2 
On Wed, Jun 18, 2014 at 2:24 PM, Kai Tietz <ktietz70@googlemail.com> wrote:

> The following change in predicates.md seems to be a bit premature.
> There is still the point about Darwin's PIC issue for unspec-gotpcrel.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61387
>
>   return ANY_QI_REG_P (op);
> })
>
> +;; Return true if OP is a memory operands that can be used in sibcalls.
> (define_predicate "sibcall_memory_operand"
> -  (match_operand 0 "memory_operand")
> -{
> -  return CONSTANT_P (XEXP (op, 0));
> -})
> +  (and (match_operand 0 "memory_operand")
> +       (match_test "CONSTANT_P (XEXP (op, 0))")))
>
> as we might to pessimize for Darwin UNSPEC_GOTPCREL at that point.
>  In general there is still the question why this issue just happens
> for Darwin, but not for linux.  For linux that gotpcrel-code path
> seems not to be hit at all (at least is that what Ians told).

Oh, this part doesn't change any functionality at all. The predicate
is just written in a different way.

Uros.
diff mbox

Patch

Index: i386.md
===================================================================
--- i386.md	(revision 211725)
+++ i386.md	(working copy)
@@ -11354,53 +11354,38 @@ 
   "* return ix86_output_call_insn (insn, operands[0]);"
   [(set_attr "type" "call")])
 
-(define_insn "*sibcall_intern"
-  [(call (unspec [(mem:QI (match_operand:W 0 "memory_nox32_operand"))]
-	   UNSPEC_PEEPSIB)
-	 (match_operand 1))]
-  ""
+(define_insn "*sibcall_memory"
+  [(call (mem:QI (match_operand:W 0 "memory_operand" "m"))
+	 (match_operand 1))
+   (unspec [(const_int 0)] UNSPEC_PEEPSIB)]
+  "!TARGET_X32"
   "* return ix86_output_call_insn (insn, operands[0]);"
   [(set_attr "type" "call")])
 
 (define_peephole2
-  [(set (match_operand:DI 0 "register_operand")
-        (match_operand:DI 1 "memory_nox32_operand"))
+  [(set (match_operand:W 0 "register_operand")
+	(match_operand:W 1 "memory_operand"))
    (call (mem:QI (match_dup 0))
          (match_operand 3))]
-  "TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))"
-  [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
-         (match_dup 3))])
+  "!TARGET_X32 && SIBLING_CALL_P (peep2_next_insn (1))
+   && peep2_reg_dead_p (2, operands[0])"
+  [(parallel [(call (mem:QI (match_dup 1))
+		    (match_dup 3))
+	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
 
 (define_peephole2
-  [(set (match_operand:DI 0 "register_operand")
-        (match_operand:DI 1 "memory_nox32_operand"))
+  [(set (match_operand:W 0 "register_operand")
+	(match_operand:W 1 "memory_operand"))
    (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
    (call (mem:QI (match_dup 0))
          (match_operand 3))]
-  "TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (2))"
+  "!TARGET_X32 && SIBLING_CALL_P (peep2_next_insn (2))
+   && peep2_reg_dead_p (3, operands[0])"
   [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
-   (call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
-         (match_dup 3))])
+   (parallel [(call (mem:QI (match_dup 1))
+		    (match_dup 3))
+	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
 
-(define_peephole2
-  [(set (match_operand:SI 0 "register_operand")
-        (match_operand:SI 1 "memory_nox32_operand"))
-   (call (mem:QI (match_dup 0))
-         (match_operand 3))]
-  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))"
-  [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
-         (match_dup 3))])
-
-(define_peephole2
-  [(set (match_operand:SI 0 "register_operand")
-        (match_operand:SI 1 "memory_nox32_operand"))
-   (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
-   (call (mem:QI (match_dup 0))
-         (match_operand 3))]
-  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (2))"
-  [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
-   (call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB) (match_dup 3))])
-
 (define_expand "call_pop"
   [(parallel [(call (match_operand:QI 0)
 		    (match_operand:SI 1))
@@ -11434,42 +11419,52 @@ 
   "* return ix86_output_call_insn (insn, operands[0]);"
   [(set_attr "type" "call")])
 
-(define_insn "*sibcall_pop_intern"
-  [(call (unspec [(mem:QI (match_operand:SI 0 "memory_nox32_operand"))]
-           UNSPEC_PEEPSIB)
+(define_insn "*sibcall_pop_memory"
+  [(call (mem:QI (match_operand:SI 0 "memory_operand" "m"))
 	 (match_operand 1))
    (set (reg:SI SP_REG)
 	(plus:SI (reg:SI SP_REG)
-		 (match_operand:SI 2 "immediate_operand" "i")))]
+		 (match_operand:SI 2 "immediate_operand" "i")))
+   (unspec [(const_int 0)] UNSPEC_PEEPSIB)]
   "!TARGET_64BIT"
   "* return ix86_output_call_insn (insn, operands[0]);"
   [(set_attr "type" "call")])
 
 (define_peephole2
   [(set (match_operand:SI 0 "register_operand")
-        (match_operand:SI 1 "memory_nox32_operand"))
+	(match_operand:SI 1 "memory_operand"))
    (parallel [(call (mem:QI (match_dup 0))
 		    (match_operand 3))
 	      (set (reg:SI SP_REG)
-		   (match_operand 4))])]
-  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))"
-  [(parallel [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
+		   (plus:SI (reg:SI SP_REG)
+			    (match_operand:SI 4 "immediate_operand")))])]
+  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))
+   && peep2_reg_dead_p (2, operands[0])"
+  [(parallel [(call (mem:QI (match_dup 1))
 		    (match_dup 3))
-	      (set (reg:SI SP_REG) (match_dup 4))])])
+	      (set (reg:SI SP_REG)
+		   (plus:SI (reg:SI SP_REG)
+			    (match_dup 4)))
+	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
 
 (define_peephole2
   [(set (match_operand:SI 0 "register_operand")
-        (match_operand:SI 1 "memory_nox32_operand"))
+	(match_operand:SI 1 "memory_operand"))
    (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
    (parallel [(call (mem:QI (match_dup 0))
 		    (match_operand 3))
 	      (set (reg:SI SP_REG)
-		   (match_operand 4))])]
-  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (2))"
+		   (plus:SI (reg:SI SP_REG)
+			    (match_operand:SI 4 "immediate_operand")))])]
+  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (2))
+   && peep2_reg_dead_p (3, operands[0])"
   [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
-   (parallel [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
+   (parallel [(call (mem:QI (match_dup 1))
 		    (match_dup 3))
-	      (set (reg:SI SP_REG) (match_dup 4))])])
+	      (set (reg:SI SP_REG)
+		   (plus:SI (reg:SI SP_REG)
+			    (match_dup 4)))
+	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
 
 ;; Call subroutine, returning value in operand 0
 
@@ -11513,63 +11508,43 @@ 
   "* return ix86_output_call_insn (insn, operands[1]);"
   [(set_attr "type" "callv")])
 
-(define_insn "*sibcall_value_intern"
+(define_insn "*sibcall_value_memory"
   [(set (match_operand 0)
-    (call (unspec [(mem:QI (match_operand:W 1 "memory_nox32_operand"))]
-	    UNSPEC_PEEPSIB)
-          (match_operand 2)))]
-  ""
+ 	(call (mem:QI (match_operand:W 1 "memory_operand" "m"))
+	      (match_operand 2)))
+   (unspec [(const_int 0)] UNSPEC_PEEPSIB)]
+  "!TARGET_X32"
   "* return ix86_output_call_insn (insn, operands[1]);"
   [(set_attr "type" "callv")])
 
 (define_peephole2
-  [(set (match_operand:DI 0 "register_operand")
-        (match_operand:DI 1 "memory_nox32_operand"))
+  [(set (match_operand:W 0 "register_operand")
+	(match_operand:W 1 "memory_operand"))
    (set (match_operand 2)
    (call (mem:QI (match_dup 0))
 		 (match_operand 3)))]
-  "TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))"
-  [(set (match_dup 2)
-   (call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
-	 (match_dup 3)))])
+  "!TARGET_X32 && SIBLING_CALL_P (peep2_next_insn (1))
+   && peep2_reg_dead_p (2, operands[0])"
+  [(parallel [(set (match_dup 2)
+		   (call (mem:QI (match_dup 1))
+			 (match_dup 3)))
+	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
 
 (define_peephole2
-  [(set (match_operand:DI 0 "register_operand")
-        (match_operand:DI 1 "memory_nox32_operand"))
+  [(set (match_operand:W 0 "register_operand")
+	(match_operand:W 1 "memory_operand"))
    (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
    (set (match_operand 2)
 	(call (mem:QI (match_dup 0))
 	      (match_operand 3)))]
-  "TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (2))"
+  "!TARGET_X32 && SIBLING_CALL_P (peep2_next_insn (2))
+   && peep2_reg_dead_p (3, operands[0])"
   [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
-   (set (match_dup 2)
-	(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
-	      (match_dup 3)))])
+   (parallel [(set (match_dup 2)
+		   (call (mem:QI (match_dup 1))
+			 (match_dup 3)))
+	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
 
-(define_peephole2
-  [(set (match_operand:SI 0 "register_operand")
-        (match_operand:SI 1 "memory_nox32_operand"))
-   (set (match_operand 2)
-	(call (mem:QI (match_dup 0))
-	      (match_operand 3)))]
-  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))"
-  [(set (match_dup 2)
-	(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
-	      (match_dup 3)))])
-
-(define_peephole2
-  [(set (match_operand:SI 0 "register_operand")
-        (match_operand:SI 1 "memory_nox32_operand"))
-   (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
-   (set (match_operand 2)
-	(call (mem:QI (match_dup 0))
-	      (match_operand 3)))]
-  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (2))"
-  [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
-   (set (match_dup 2)
-	(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
-	      (match_dup 3)))])
-
 (define_insn "*call_value_rex64_ms_sysv"
   [(match_parallel 3 "call_rex64_ms_sysv_operation"
     [(set (match_operand 0)
@@ -11616,55 +11591,57 @@ 
   "* return ix86_output_call_insn (insn, operands[1]);"
   [(set_attr "type" "callv")])
 
-(define_insn "*sibcall_value_pop_intern"
+(define_insn "*sibcall_value_pop_memory"
   [(set (match_operand 0)
-        (call (unspec [(mem:QI (match_operand:SI 1 "memory_nox32_operand"))]
-	       UNSPEC_PEEPSIB)
+ 	(call (mem:QI (match_operand:SI 1 "memory_operand" "m"))
 	  (match_operand 2)))
    (set (reg:SI SP_REG)
 	(plus:SI (reg:SI SP_REG)
-		 (match_operand:SI 3 "immediate_operand" "i")))]
+		 (match_operand:SI 3 "immediate_operand" "i")))
+   (unspec [(const_int 0)] UNSPEC_PEEPSIB)]
   "!TARGET_64BIT"
   "* return ix86_output_call_insn (insn, operands[1]);"
   [(set_attr "type" "callv")])
 
 (define_peephole2
   [(set (match_operand:SI 0 "register_operand")
-        (match_operand:SI 1 "memory_nox32_operand"))
+	(match_operand:SI 1 "memory_operand"))
    (parallel [(set (match_operand 2)
 	      (call (mem:QI (match_dup 0))
 		    (match_operand 3)))
 	    (set (reg:SI SP_REG)
 		(plus:SI (reg:SI SP_REG)
-			  (match_operand:SI 4 "immediate_operand")))]
-   )]
-  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))"
+			    (match_operand:SI 4 "immediate_operand")))])]
+  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))
+   && peep2_reg_dead_p (2, operands[0])"
   [(parallel [(set (match_dup 2)
-	      (call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
+		   (call (mem:QI (match_dup 1))
 		    (match_dup 3)))
 	      (set (reg:SI SP_REG)
 		   (plus:SI (reg:SI SP_REG)
-			    (match_dup 4)))])])
+			    (match_dup 4)))
+	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
 
 (define_peephole2
   [(set (match_operand:SI 0 "register_operand")
-        (match_operand:SI 1 "memory_nox32_operand"))
+	(match_operand:SI 1 "memory_operand"))
    (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
    (parallel [(set (match_operand 2)
 	      (call (mem:QI (match_dup 0))
 		    (match_operand 3)))
 	    (set (reg:SI SP_REG)
 		(plus:SI (reg:SI SP_REG)
-			  (match_operand:SI 4 "immediate_operand")))]
-   )]
-  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (1))"
+			    (match_operand:SI 4 "immediate_operand")))])]
+  "!TARGET_64BIT && SIBLING_CALL_P (peep2_next_insn (2))
+   && peep2_reg_dead_p (3, operands[0])"
   [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE)
    (parallel [(set (match_dup 2)
-	      (call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB)
+		   (call (mem:QI (match_dup 1))
 		    (match_dup 3)))
 	      (set (reg:SI SP_REG)
 		   (plus:SI (reg:SI SP_REG)
-			    (match_dup 4)))])])
+			    (match_dup 4)))
+	      (unspec [(const_int 0)] UNSPEC_PEEPSIB)])])
 
 ;; Call subroutine returning any type.
 
Index: predicates.md
===================================================================
--- predicates.md	(revision 211725)
+++ predicates.md	(working copy)
@@ -71,11 +71,10 @@ 
   return ANY_QI_REG_P (op);
 })
 
+;; Return true if OP is a memory operands that can be used in sibcalls.
 (define_predicate "sibcall_memory_operand"
-  (match_operand 0 "memory_operand")
-{
-  return CONSTANT_P (XEXP (op, 0));
-})
+  (and (match_operand 0 "memory_operand")
+       (match_test "CONSTANT_P (XEXP (op, 0))")))
 
 ;; Match an SI or HImode register for a zero_extract.
 (define_special_predicate "ext_register_operand"
@@ -587,11 +586,6 @@ 
   (ior (match_operand 0 "register_no_elim_operand")
        (match_operand 0 "immediate_operand")))
 
-;; Test for a valid memory operand.
-(define_predicate "memory_nox32_operand"
-  (and (not (match_test "TARGET_X32"))
-       (match_operand 0 "memory_operand")))
-
 ;; Test for a valid operand for indirect branch.
 (define_predicate "indirect_branch_operand"
   (ior (match_operand 0 "register_operand")