public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Dorit Naishlos <DORIT@il.ibm.com>
To: gcc-patches@gcc.gnu.org
Cc: mark@codesourcery.com, Richard Henderson <rth@redhat.com>
Subject: [patch] vectorizer: fix handling of non VECTOR_MODE_P vectypes
Date: Mon, 11 Oct 2004 22:48:00 -0000	[thread overview]
Message-ID: <OFEA1087A3.A6AD6985-ONC2256F2A.0045003B-C2256F2A.0078F91F@il.ibm.com> (raw)
In-Reply-To: <415A7DC5.2000303@gnu.org>

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


> > so it should return a V2SI vectype, which is not supported by the
target
> > and therefore should have a BLKmode. no?
>
> Nope.  An integer mode is returned if we have one that fits.  It
> makes the routines in tree-complex.c happy.

In this case we also need to fix the computation of the vectorization
factor - currently it is set to GET_MODE_NUNITS, which is correct only if
the mode is VECTOR_MODE_P. This came up on ppc-darwin compiling SPEC's
crafty - build_vector_type returned TImode for a 'long long' type, and the
vectorization factor was set to 1 instead of 2. Crafty failed as a result.
(Ideally build_vector_type would have returned BLKmode rather than TImode;
see machmode.def: "FIXME TI shouldn't be generically available either.  *".
I didn't fail vectorization in this case, although this type is not really
supported and the vectorization will be undone later on).

Bootstrapped and tested on ppc-darwin. ok for mainline?

thanks,
dorit

        * tree-vectorizer.c (get_vectype_for_scalar_type): Added debug
printouts.
        Added checks under ENABLE_CHECKING to verify the vectorizer's
        assumptions on the vectype returned by build_vector_type. Check if
TImode.
        (vect_transform_loop): Added gcc_assert under ENABLE_CHECKING.
        (vect_analyze_operations): Don't use GET_MODE_NUNITS to determine
the
        vectorization factor. Make sure the vectorization factor > 1. Add
        gcc_assert under ENABLE_CHECKING.
        (vect_get_vec_def_for_operand): Remove redundant variables.

(See attached file: vectfix2)





|---------+---------------------------->
|         |           Paolo Bonzini    |
|         |           <bonzini@gnu.org>|
|         |                            |
|         |           29/09/2004 11:17 |
|---------+---------------------------->
  >----------------------------------------------------------------------------------------------------------------------------|
  |                                                                                                                            |
  |       To:       Paolo Bonzini <bonzini@gnu.org>, Richard Henderson <rth@redhat.com>, Dorit Naishlos/Haifa/IBM@IBMIL        |
  |       cc:       Chao-ying Fu <fu@mips.com>, gcc-patches@gcc.gnu.org, "Stephens, Nigel" <nigel@mercury.mips.com>, "Thekkath,|
  |        Radhika" <radhika@mercury.mips.com>, Richard Sandiford <rsandifo@redhat.com>, "Uhler, Mike"                         |
  |        <uhler@mercury.mips.com>, Jim Wilson <wilson@specifixinc.com>                                                       |
  |       Subject:  Re: [patch] extend.texi MIPS PS/3D Support                                                                 |
  >----------------------------------------------------------------------------------------------------------------------------|




> Yes, it should.  Generic vectors should not be created by the vectorizer
> (yet).

Ah, I thought that the vectorizer created its vector types with a
separate routine, but that has been fixed a while ago.  The attached
patch should help.

It was bootstrapped and passes the vectorizer testsuite.  Full regtest
on i686-pc-linux-gnu is in progress.  Ok for mainline?

Paolo

2004-09-29  Paolo Bonzini  <bonzini@gnu.org>

             * tree-vectorizer.c (vectorizable_operation): Only look at the
             optab in the case of vector modes; otherwise, bail out.
Index: tree-vectorizer.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-vectorizer.c,v
retrieving revision 2.12
diff -u -r2.12 tree-vectorizer.c
--- tree-vectorizer.c          25 Sep 2004 14:48:03 -0000          2.12
+++ tree-vectorizer.c          29 Sep 2004 07:31:24 -0000
@@ -1408,6 +1408,17 @@
       return false;
     }
   vec_mode = TYPE_MODE (vectype);
+  if (GET_MODE_CLASS (vec_mode) != MODE_VECTOR_INT
+      && GET_MODE_CLASS (vec_mode) != MODE_VECTOR_FLOAT)
+    {
+      /* TODO: tree-complex.c sometimes can parallelize operations
+             on generic vectors.  We can vectorize the loop in that case,
+             but then we should re-run the lowering pass.  */
+      if (vect_debug_details (NULL))
+            fprintf (dump_file, "vector mode not supported by target.");
+      return false;
+    }
+
   if (optab->handlers[(int) vec_mode].insn_code == CODE_FOR_nothing)
     {
       if (vect_debug_details (NULL))


[-- Attachment #2: vectfix2 --]
[-- Type: application/octet-stream, Size: 7044 bytes --]

Index: tree-vectorizer.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-vectorizer.c,v
retrieving revision 2.14
diff -c -3 -p -r2.14 tree-vectorizer.c
*** tree-vectorizer.c	1 Oct 2004 09:59:00 -0000	2.14
--- tree-vectorizer.c	11 Oct 2004 17:49:35 -0000
*************** static tree
*** 836,841 ****
--- 836,842 ----
  get_vectype_for_scalar_type (tree scalar_type)
  {
    enum machine_mode inner_mode = TYPE_MODE (scalar_type);
+   enum machine_mode vec_mode;
    int nbytes = GET_MODE_SIZE (inner_mode);
    int nunits;
    tree vectype;
*************** get_vectype_for_scalar_type (tree scalar
*** 848,855 ****
    nunits = UNITS_PER_SIMD_WORD / nbytes;
  
    vectype = build_vector_type (scalar_type, nunits);
!   if (TYPE_MODE (vectype) == BLKmode)
      return NULL_TREE;
    return vectype;
  }
  
--- 849,916 ----
    nunits = UNITS_PER_SIMD_WORD / nbytes;
  
    vectype = build_vector_type (scalar_type, nunits);
!   if (vect_debug_details (NULL))
!     {
!       fprintf (dump_file, "get vectype with %d units of type ", nunits);
!       print_generic_expr (dump_file, scalar_type, TDF_SLIM);
!     }
! 
!   if (!vectype)
      return NULL_TREE;
+ 
+   if (vect_debug_details (NULL))
+     {
+       fprintf (dump_file, "vectype: ");
+       print_generic_expr (dump_file, vectype, TDF_SLIM);
+     }
+ 
+   vec_mode = TYPE_MODE (vectype);
+ 
+   /* Is type supported by target?:  */
+   if (vec_mode == BLKmode)
+     {
+       if (vect_debug_details (NULL))
+         fprintf (dump_file, "vectype not supported by target.");
+       return NULL_TREE;
+     }
+ 
+   if (vec_mode == TImode)
+     {
+       /* See comment in machmode.def:
+          "FIXME TI shouldn't be generically available either."
+          The vectorizer will report that it successfully vectorized,
+          but this will actually be lowered during RTL expand. */
+       if (vect_debug_details (NULL))
+         fprintf (dump_file, "TImode; not really supported by target.");
+       /* return NULL_TREE; */ /* CHECKME */
+     }
+ 
+ #ifdef ENABLE_CHECKING
+   if (GET_MODE_SIZE (vec_mode) != UNITS_PER_SIMD_WORD)
+     {
+       if (vect_debug_details (NULL))
+         fprintf (dump_file, "unexpected vectype size.");
+       return NULL_TREE;
+     }
+ 
+   if (VECTOR_MODE_P (vec_mode))
+     {
+       if (nbytes * GET_MODE_NUNITS (vec_mode) != UNITS_PER_SIMD_WORD)
+         {
+           if (vect_debug_details (NULL))
+             fprintf (dump_file, "unexpected vectype nunits: nunits = %d",
+                      GET_MODE_NUNITS (vec_mode));
+           return NULL_TREE;
+         }
+       else
+         if (vect_debug_details (NULL))
+           fprintf (dump_file, "supportable vector type.");
+     }
+   else
+     if (vect_debug_details (NULL))
+       fprintf (dump_file, "type not really a vector.");
+ #endif
+ 
    return vectype;
  }
  
*************** vect_get_vec_def_for_operand (tree op, t
*** 1157,1167 ****
        /* Create 'vect_cst_ = {cst,cst,...,cst}'  */
  
        tree vec_cst;
-       stmt_vec_info stmt_vinfo = vinfo_for_stmt (stmt);
-       tree vectype = STMT_VINFO_VECTYPE (stmt_vinfo);
-       int nunits = GET_MODE_NUNITS (TYPE_MODE (vectype));
-       tree t = NULL_TREE;
-       int i;
  
        /* Build a tree with vector elements.  */
        if (vect_debug_details (NULL))
--- 1218,1223 ----
*************** vect_transform_loop (loop_vec_info loop_
*** 1907,1912 ****
--- 1963,1969 ----
  	  bool is_store;
  #ifdef ENABLE_CHECKING
  	  tree vectype;
+ 	  enum machine_mode vecmode;
  #endif
  
  	  if (vect_debug_details (NULL))
*************** vect_transform_loop (loop_vec_info loop_
*** 1925,1932 ****
  	  /* FORNOW: Verify that all stmts operate on the same number of
  	             units and no inner unrolling is necessary.  */
  	  vectype = STMT_VINFO_VECTYPE (stmt_info);
! 	  gcc_assert (GET_MODE_NUNITS (TYPE_MODE (vectype))
! 		      == vectorization_factor);
  #endif
  	  /* -------- vectorize statement ------------ */
  	  if (vect_debug_details (NULL))
--- 1982,1991 ----
  	  /* FORNOW: Verify that all stmts operate on the same number of
  	             units and no inner unrolling is necessary.  */
  	  vectype = STMT_VINFO_VECTYPE (stmt_info);
! 	  vecmode = TYPE_MODE (vectype);
! 	  gcc_assert (GET_MODE_SIZE (vecmode) == UNITS_PER_SIMD_WORD);
! 	  gcc_assert (GET_MODE_NUNITS (vecmode) == vectorization_factor
! 		      || !VECTOR_MODE_P (vecmode));
  #endif
  	  /* -------- vectorize statement ------------ */
  	  if (vect_debug_details (NULL))
*************** vect_analyze_operations (loop_vec_info l
*** 2044,2050 ****
    int vectorization_factor = 0;
    int i;
    bool ok;
-   tree scalar_type;
  
    if (vect_debug_details (NULL))
      fprintf (dump_file, "\n<<vect_analyze_operations>>\n");
--- 2103,2108 ----
*************** vect_analyze_operations (loop_vec_info l
*** 2059,2064 ****
--- 2117,2124 ----
  	  int nunits;
  	  stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
  	  tree vectype;
+ 	  tree scalar_type;
+ 	  int scalar_size;
  
  	  if (vect_debug_details (NULL))
  	    {
*************** vect_analyze_operations (loop_vec_info l
*** 2138,2146 ****
  	      return false;
  	    }
  
! 	  nunits = GET_MODE_NUNITS (TYPE_MODE (vectype));
! 	  if (vect_debug_details (NULL))
! 	    fprintf (dump_file, "nunits = %d", nunits);
  
  	  if (vectorization_factor)
  	    {
--- 2198,2216 ----
  	      return false;
  	    }
  
!           /* Note that:
!              (*) GET_MODE_NUNITS (TYPE_MODE (vectype)) == vf
!              is not necessarily true! It only holds if the mode of vectype
! 	       is VECTOR_MODE_P.  E.g, V4QI may be represented with SI;
!              in this case, the LHS is 1, but the vectorization factor (vf)
!              should be 4.
! 
!              We don't require that mode be always VECTOR_TYPE_P because we 
! 	       can vectorize such datatypes in some cases (for example, if there 
! 	       are only data movements).  */
! 
! 	  scalar_size = GET_MODE_SIZE (TYPE_MODE (scalar_type));
! 	  nunits = UNITS_PER_SIMD_WORD / scalar_size;
  
  	  if (vectorization_factor)
  	    {
*************** vect_analyze_operations (loop_vec_info l
*** 2154,2165 ****
  		}
  	    }
  	  else
! 	    vectorization_factor = nunits;
  	}
      }
  
    /* TODO: Analyze cost. Decide if worth while to vectorize.  */
!   if (!vectorization_factor)
      {
        if (vect_debug_stats (loop) || vect_debug_details (loop))
          fprintf (dump_file, "not vectorized: unsupported data-type");
--- 2224,2240 ----
  		}
  	    }
  	  else
!             vectorization_factor = nunits;
! 
! #ifdef ENABLE_CHECKING
!           gcc_assert (scalar_size * vectorization_factor == UNITS_PER_SIMD_WORD);
! #endif
  	}
      }
  
    /* TODO: Analyze cost. Decide if worth while to vectorize.  */
! 
!   if (vectorization_factor <= 1)
      {
        if (vect_debug_stats (loop) || vect_debug_details (loop))
          fprintf (dump_file, "not vectorized: unsupported data-type");

  parent reply	other threads:[~2004-10-11 22:05 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-09-23 18:02 [patch] extend.texi MIPS PS/3D Support Fu, Chao-Ying
2004-09-24 12:17 ` Dorit Naishlos
2004-09-24 21:35   ` Chao-ying Fu
2004-09-25 10:53     ` Richard Sandiford
2004-09-27 23:32       ` Chao-ying Fu
2004-09-28  0:32         ` Dorit Naishlos
2004-09-28  0:41           ` Richard Henderson
2004-09-28 14:07             ` Dorit Naishlos
2004-09-28 14:08               ` Paolo Bonzini
2004-09-29 11:41                 ` Paolo Bonzini
2004-09-29 17:33                   ` Richard Henderson
2004-09-29 23:39                   ` Dorit Naishlos
2004-10-11 22:48                   ` Dorit Naishlos [this message]
2004-10-12  0:40                     ` [patch] vectorizer: fix handling of non VECTOR_MODE_P vectypes Richard Henderson
2004-10-12 12:54                       ` Dorit Naishlos
2004-10-12 16:37                         ` Richard Henderson
2004-10-12 18:21                           ` Dorit Naishlos
2004-10-12 20:49                             ` James A. Morrison
2004-10-13  8:46                               ` Dorit Naishlos
2004-10-13 19:18                         ` Richard Henderson
2004-09-28 19:35               ` [patch] extend.texi MIPS PS/3D Support Chao-ying Fu
2004-09-28 23:04               ` Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=OFEA1087A3.A6AD6985-ONC2256F2A.0045003B-C2256F2A.0078F91F@il.ibm.com \
    --to=dorit@il.ibm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=mark@codesourcery.com \
    --cc=rth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).