Re: [PATCH] Vzeroupper placement/47440

[Uros Bizjak <ubizjak@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]

On Wed, Nov 7, 2012 at 9:04 AM, Jakub Jelinek <jakub@redhat.com> wrote:

> Or I wonder why is call handled specially at all, doesn't
>   /* Check if a 256bit AVX register is referenced in stores.  */
>   state = unused;
>   note_stores (pat, check_avx256_stores, &state);
>   if (state == used)
>     return AVX_U128_DIRTY;
> handle it?  Then it would just need to be if (CALL_P (insn)) return AVX_U128_CLEAN.
> BTW, the formatting is wrong in some spots, e.g.
> check_avx256_stores (rtx dest, const_rtx set, void *data)
> {
>   if (((REG_P (dest) || MEM_P(dest))
>
> I'd prefer to leave this to the original submitter.

I have committed following patch that address all the above issues.

2012-11-07  Uros Bizjak  <ubizjak@gmail.com>

	* config/i386/i386.c (enum upper_128bits_state): Remove.
	(check_avx256_store): Use bool pointer argument.
	(ix86_avx_u128_mode_needed): Use note_stores also for CALL insns.
	* config/i386/predicates.md (vzeroupper_operation): Use match_test.

Bootstrapped and regression tested on x86_64-pc-linux-gnu, committed.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 4678 bytes --]

Index: predicates.md
===================================================================
--- predicates.md	(revision 193292)
+++ predicates.md	(working copy)
@@ -1231,10 +1231,8 @@
 
 ;; return true if OP is a vzeroupper operation.
 (define_predicate "vzeroupper_operation"
-  (match_code "unspec_volatile")
-{
-  return XINT (op, 1) == UNSPECV_VZEROUPPER;
-})
+  (and (match_code "unspec_volatile")
+       (match_test "XINT (op, 1) == UNSPECV_VZEROUPPER")))
 
 ;; Return true if OP is a parallel for a vbroadcast permute.
 
Index: i386.c
===================================================================
--- i386.c	(revision 193292)
+++ i386.c	(working copy)
@@ -65,27 +65,19 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-pass.h"
 #include "tree-flow.h"
 
-enum upper_128bits_state
-{
-  unknown = 0,
-  unused,
-  used
-};
-
 /* Check if a 256bit AVX register is referenced in stores.   */
 
 static void
 check_avx256_stores (rtx dest, const_rtx set, void *data)
 {
-  if (((REG_P (dest) || MEM_P(dest))
+  if (((REG_P (dest) || MEM_P (dest))
        && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (dest)))
       || (GET_CODE (set) == SET
 	  && (REG_P (SET_SRC (set)) || MEM_P (SET_SRC (set)))
 	  && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (SET_SRC (set)))))
     {
-      enum upper_128bits_state *state
-	= (enum upper_128bits_state *) data;
-      *state = used;
+      bool *used = (bool *) data;
+      *used = true;
     }
 }
 
@@ -14967,23 +14959,24 @@ output_387_binary_op (rtx insn, rtx *operands)
 static int
 ix86_avx_u128_mode_needed (rtx insn)
 {
-  rtx pat = PATTERN (insn);
-  rtx arg;
-  enum upper_128bits_state state;
+  bool avx_u128_used;
 
   if (CALL_P (insn))
     {
+      rtx link;
+
       /* Needed mode is set to AVX_U128_CLEAN if there are
 	 no 256bit modes used in function arguments.  */
-      for (arg = CALL_INSN_FUNCTION_USAGE (insn); arg;
-	   arg = XEXP (arg, 1))
+      for (link = CALL_INSN_FUNCTION_USAGE (insn);
+	   link;
+	   link = XEXP (link, 1))
 	{
-	  if (GET_CODE (XEXP (arg, 0)) == USE)
+	  if (GET_CODE (XEXP (link, 0)) == USE)
 	    {
-	      rtx reg = XEXP (XEXP (arg, 0), 0);
+	      rtx arg = XEXP (XEXP (link, 0), 0);
 
-	      if (reg && REG_P (reg)
-		  && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (reg)))
+	      if (REG_P (arg)
+		  && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (arg)))
 		return AVX_U128_ANY;
 	    }
 	}
@@ -14992,10 +14985,11 @@ ix86_avx_u128_mode_needed (rtx insn)
     }
 
   /* Check if a 256bit AVX register is referenced in stores.  */
-  state = unused;
-  note_stores (pat, check_avx256_stores, &state);
-  if (state == used)
+  avx_u128_used = false;
+  note_stores (PATTERN (insn), check_avx256_stores, &avx_u128_used);
+  if (avx_u128_used)
     return AVX_U128_DIRTY;
+
   return AVX_U128_ANY;
 }
 
@@ -15079,39 +15073,21 @@ static int
 ix86_avx_u128_mode_after (int mode, rtx insn)
 {
   rtx pat = PATTERN (insn);
-  rtx reg = NULL;
-  int i;
-  enum upper_128bits_state state;
+  bool avx_u128_used;
 
-  /* Check for CALL instruction.  */
-  if (CALL_P (insn))
-    {
-      if (GET_CODE (pat) == SET)
-	reg = SET_DEST (pat);
-      else if (GET_CODE (pat) == PARALLEL)
-	for (i = XVECLEN (pat, 0) - 1; i >= 0; i--)
-	  {
-	    rtx x = XVECEXP (pat, 0, i);
-	    if (GET_CODE(x) == SET)
-	      reg = SET_DEST (x);
-	  }
-      /* Mode after call is set to AVX_U128_DIRTY if there are
-	 256bit modes used in the function return register.  */
-      if (reg && REG_P (reg) && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (reg)))
-	return AVX_U128_DIRTY;
-      else
-	return AVX_U128_CLEAN;
-    }
-
   if (vzeroupper_operation (pat, VOIDmode)
       || vzeroall_operation (pat, VOIDmode))
     return AVX_U128_CLEAN;
 
   /* Check if a 256bit AVX register is referenced in stores.  */
-  state = unused;
-  note_stores (pat, check_avx256_stores, &state);
-  if (state == used)
+  avx_u128_used = false;
+  note_stores (pat, check_avx256_stores, &avx_u128_used);
+  if (avx_u128_used)
     return AVX_U128_DIRTY;
+  /* We know that state is clean after CALL insn if there are no
+     256bit modes used in the function return register.  */
+  else if (CALL_P (insn))
+    return AVX_U128_CLEAN;
 
   return mode;
 }
@@ -15145,9 +15121,10 @@ ix86_avx_u128_mode_entry (void)
   for (arg = DECL_ARGUMENTS (current_function_decl); arg;
        arg = TREE_CHAIN (arg))
     {
-      rtx reg = DECL_INCOMING_RTL (arg);
+      rtx incoming = DECL_INCOMING_RTL (arg);
 
-      if (reg && REG_P (reg) && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (reg)))
+      if (incoming && REG_P (incoming)
+	  && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (incoming)))
 	return AVX_U128_DIRTY;
     }