This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Altivec: Fix vmx/varargs-7.c


The (not yet committed -- see my message on Friday) test case
gcc.dg/vmx/varargs-7.c fails due to an unrecognizable insn at any
level of optimization.  This turns out to be because
function.c:instantiate_decl() was never updated for vector modes.

The bug, in short, is that an insn of the form

(set (reg:V4SI pseudo)
     (mem:V4SI (reg:SI virtual-incoming-args)))

is transformed into

(set (reg:V4SI pseudo)
     (mem:V4SI (plus:SI (reg:SI ap)
                        (const_int 8))))

Altivec load-vector instructions do not accept reg+imm addressing
(only reg and reg+reg).  Thus, memory_address_p(V4SImode, mem) fails
for that MEM.  However, PPC scalar load instructions *do* accept
reg+imm addressing, which means that the test in instantiate_decl:

   /* Now verify that the resulting address is valid for every integer or
      floating-point mode up to and including SIZE bytes long.  We do this
      since the object might be accessed in any mode and frame addresses
      are shared.  */

succeeds -- it simply does not bother checking vector modes.

My proposed patch for this bug is to nuke the initial call to
instantiate_decls() from instantiate_virtual_regs().  We are going to
call instantiate_virtual_regs_1 immediately after that; it's going to
walk the entire insn chain; it gets this case right; we might as well
let it do all the work.

I do not believe this will cause any problems.  There might be a
concern over a stack slot recycled for two different modes, one with
stricter addressing requirements, being modified in place by
instantiate_virtual_regs_1; but I claim that we would have a problem
with that case already, because all that instantiate_decls does if it
encounters such a case is punt to instantiate_virtual_regs_1.

The second call to instantiate_decls is still necessary, so that
DECL_RTL for every decl is in a form acceptable to the debug-info
generators; however, we can rip out all the code associated with
instantiate_decls being called with valid_only = 1.

I've verified that this fixes varargs-7.c.  i686-linux bootstrap is in
progress; once I straighten out some issues with my simulator harness
I will be doing a complete build and testsuite run for an
i686-linux->powerpc-eabisimaltivec cross compile configuration as
well, including the 'vmx' test bundle.

OK to commit, assuming testing is successful?

zw

        * function.c (instantiate_virtual_regs): Remove first call to
        instantiate_decls.  Delete second argument to second call.
        (instantiate_virtual_regs_1): Update a comment.
        (instantiate_decls, instantiate_decls_1, instantiate-decl):
        Now only called in the valid_only=0 condition, so nuke that
        argument, the 'size' argument to instantiate_decl, and all
        code only relevant to valid_only=1.

===================================================================
Index: function.c
--- function.c	14 Oct 2002 21:19:04 -0000	1.387
+++ function.c	27 Nov 2002 00:20:48 -0000
@@ -247,9 +247,9 @@ static rtx walk_fixup_memory_subreg  PAR
 					      int));
 static rtx fixup_stack_1	PARAMS ((rtx, rtx));
 static void optimize_bit_field	PARAMS ((rtx, rtx, rtx *));
-static void instantiate_decls	PARAMS ((tree, int));
-static void instantiate_decls_1	PARAMS ((tree, int));
-static void instantiate_decl	PARAMS ((rtx, HOST_WIDE_INT, int));
+static void instantiate_decls	PARAMS ((tree));
+static void instantiate_decls_1	PARAMS ((tree));
+static void instantiate_decl	PARAMS ((rtx));
 static rtx instantiate_new_reg	PARAMS ((rtx, HOST_WIDE_INT *));
 static int instantiate_virtual_regs_1 PARAMS ((rtx *, rtx, int));
 static void delete_handlers	PARAMS ((void));
@@ -3546,12 +3546,6 @@ instantiate_virtual_regs (fndecl, insns)
   out_arg_offset = STACK_POINTER_OFFSET;
   cfa_offset = ARG_POINTER_CFA_OFFSET (fndecl);
 
-  /* Scan all variables and parameters of this function.  For each that is
-     in memory, instantiate all virtual registers if the result is a valid
-     address.  If not, we do it later.  That will handle most uses of virtual
-     regs on many machines.  */
-  instantiate_decls (fndecl, 1);
-
   /* Initialize recognition, indicating that volatile is OK.  */
   init_recog ();
 
@@ -3577,7 +3571,7 @@ instantiate_virtual_regs (fndecl, insns)
 
   /* Now instantiate the remaining register equivalences for debugging info.
      These will not be valid addresses.  */
-  instantiate_decls (fndecl, 0);
+  instantiate_decls (fndecl);
 
   /* Indicate that, from now on, assign_stack_local should use
      frame_pointer_rtx.  */
@@ -3587,70 +3581,51 @@ instantiate_virtual_regs (fndecl, insns)
 /* Scan all decls in FNDECL (both variables and parameters) and instantiate
    all virtual registers in their DECL_RTL's.
 
-   If VALID_ONLY, do this only if the resulting address is still valid.
-   Otherwise, always do it.  */
+   This is done even if the result is not valid RTL for purposes of
+   code generation, because the only use of this function is to
+   prevent virtual registers from appearing in debugging information,
+   so it doesn't matter.  All RTL actually used in code generation has
+   already been fixed up by instantiate_virtual_regs.  */
 
 static void
-instantiate_decls (fndecl, valid_only)
+instantiate_decls (fndecl)
      tree fndecl;
-     int valid_only;
 {
   tree decl;
 
   /* Process all parameters of the function.  */
   for (decl = DECL_ARGUMENTS (fndecl); decl; decl = TREE_CHAIN (decl))
-    {
-      HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (decl));
-      HOST_WIDE_INT size_rtl;
-
-      instantiate_decl (DECL_RTL (decl), size, valid_only);
-
-      /* If the parameter was promoted, then the incoming RTL mode may be
-	 larger than the declared type size.  We must use the larger of
-	 the two sizes.  */
-      size_rtl = GET_MODE_SIZE (GET_MODE (DECL_INCOMING_RTL (decl)));
-      size = MAX (size_rtl, size);
-      instantiate_decl (DECL_INCOMING_RTL (decl), size, valid_only);
-    }
+    instantiate_decl (DECL_RTL (decl));
 
   /* Now process all variables defined in the function or its subblocks.  */
-  instantiate_decls_1 (DECL_INITIAL (fndecl), valid_only);
+  instantiate_decls_1 (DECL_INITIAL (fndecl));
 }
 
 /* Subroutine of instantiate_decls: Process all decls in the given
    BLOCK node and all its subblocks.  */
 
 static void
-instantiate_decls_1 (let, valid_only)
+instantiate_decls_1 (let)
      tree let;
-     int valid_only;
 {
   tree t;
 
   for (t = BLOCK_VARS (let); t; t = TREE_CHAIN (t))
     if (DECL_RTL_SET_P (t))
-      instantiate_decl (DECL_RTL (t),
-			int_size_in_bytes (TREE_TYPE (t)),
-			valid_only);
+      instantiate_decl (DECL_RTL (t));
 
   /* Process all subblocks.  */
   for (t = BLOCK_SUBBLOCKS (let); t; t = TREE_CHAIN (t))
-    instantiate_decls_1 (t, valid_only);
+    instantiate_decls_1 (t);
 }
 
 /* Subroutine of the preceding procedures: Given RTL representing a
-   decl and the size of the object, do any instantiation required.
-
-   If VALID_ONLY is nonzero, it means that the RTL should only be
-   changed if the new address is valid.  */
+   decl and the size of the object, do any instantiation required.  */
 
 static void
-instantiate_decl (x, size, valid_only)
+instantiate_decl (x)
      rtx x;
-     HOST_WIDE_INT size;
-     int valid_only;
 {
-  enum machine_mode mode;
   rtx addr;
 
   /* If this is not a MEM, no need to do anything.  Similarly if the
@@ -3667,42 +3642,7 @@ instantiate_decl (x, size, valid_only)
 	      || REGNO (addr) > LAST_VIRTUAL_REGISTER)))
     return;
 
-  /* If we should only do this if the address is valid, copy the address.
-     We need to do this so we can undo any changes that might make the
-     address invalid.  This copy is unfortunate, but probably can't be
-     avoided.  */
-
-  if (valid_only)
-    addr = copy_rtx (addr);
-
-  instantiate_virtual_regs_1 (&addr, NULL_RTX, 0);
-
-  if (valid_only && size >= 0)
-    {
-      unsigned HOST_WIDE_INT decl_size = size;
-
-      /* Now verify that the resulting address is valid for every integer or
-	 floating-point mode up to and including SIZE bytes long.  We do this
-	 since the object might be accessed in any mode and frame addresses
-	 are shared.  */
-
-      for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
-	   mode != VOIDmode && GET_MODE_SIZE (mode) <= decl_size;
-	   mode = GET_MODE_WIDER_MODE (mode))
-	if (! memory_address_p (mode, addr))
-	  return;
-
-      for (mode = GET_CLASS_NARROWEST_MODE (MODE_FLOAT);
-	   mode != VOIDmode && GET_MODE_SIZE (mode) <= decl_size;
-	   mode = GET_MODE_WIDER_MODE (mode))
-	if (! memory_address_p (mode, addr))
-	  return;
-    }
-
-  /* Put back the address now that we have updated it and we either know
-     it is valid or we don't care whether it is valid.  */
-
-  XEXP (x, 0) = addr;
+  instantiate_virtual_regs_1 (&XEXP (x, 0), NULL_RTX, 0);
 }
 
 /* Given a piece of RTX and a pointer to a HOST_WIDE_INT, if the RTX
@@ -3960,10 +3900,9 @@ instantiate_virtual_regs_1 (loc, object,
       goto restart;
 
     case MEM:
-      /* Most cases of MEM that convert to valid addresses have already been
-	 handled by our scan of decls.  The only special handling we
-	 need here is to make a copy of the rtx to ensure it isn't being
-	 shared if we have to change it to a pseudo.
+      /* The only special handling we need here is to make a copy of
+	 the rtx to ensure it isn't being shared if we have to change
+	 it to a pseudo.
 
 	 If the rtx is a simple reference to an address via a virtual register,
 	 it can potentially be shared.  In such cases, first try to make it


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]