Combine deletes non noop insns

Combine deletes non noop insns

[John David Anglin]

While working on a patch to pass small structs in their proper location,
I have come up with the following problem.  It appears combine incorrectly
decides the following rtl is a noop:

(insn 41 40 42 0 (nil) (set (reg:QI 125)
        (mem/s:QI (plus:SI (reg/f:SI 3 %r3)
                (const_int -40 [0xffffffd8])) [0+0 S1 A8])) 61 {*pa.md:2805} (nil)
    (nil))

(insn 42 41 43 0 (nil) (set (zero_extract:SI (subreg:SI (reg:QI 125) 0)
            (const_int 8 [0x8])
            (const_int 24 [0x18]))
        (reg:SI 97)) 220 {insv_32} (insn_list 4 (insn_list 41 (nil)))
    (expr_list:REG_DEAD (reg:SI 97)
        (nil)))

(insn 43 42 44 0 (nil) (set (mem/s:QI (plus:SI (reg/f:SI 3 %r3)
                (const_int -40 [0xffffffd8])) [0+0 S1 A8])
        (reg:QI 125)) 61 {*pa.md:2805} (insn_list 42 (nil))
    (expr_list:REG_DEAD (reg:QI 125)
        (nil)))

The rtl is from the life pass.  SI 97 is set from the first argument
register and its value is not known.  While I can see that this
sequence can be simplified, the set in insn 43 can't be eliminated.

This rtl was generated with the following test program:

struct S5 { unsigned char i[5]; };
extern void check5 (struct S5 *, int);
void test5 (struct S5 s5)
{
  check5 (&s5, 64);
}

It is a simplification from struct-by-value-1.c.

struct-by-value-1.c is the only remaining struct problem left on the
32-bit port when the patch below is applied.  It passes at -O0 but not
at higher optimization levels.

Anybody got any ideas why the above rtl confuses combine?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Index: function.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/function.c,v
retrieving revision 1.381
diff -u -3 -p -r1.381 function.c
--- function.c	29 Aug 2002 19:20:00 -0000	1.381
+++ function.c	10 Sep 2002 16:36:45 -0000
@@ -255,10 +255,8 @@ static int instantiate_virtual_regs_1 PA
 static void delete_handlers	PARAMS ((void));
 static void pad_to_arg_alignment PARAMS ((struct args_size *, int,
 					  struct args_size *));
-#ifndef ARGS_GROW_DOWNWARD
 static void pad_below		PARAMS ((struct args_size *, enum machine_mode,
 					 tree));
-#endif
 static rtx round_trampoline_addr PARAMS ((rtx));
 static rtx adjust_trampoline_addr PARAMS ((rtx));
 static tree *identify_blocks_1	PARAMS ((rtx, tree *, tree *, tree *));
@@ -5244,6 +5242,9 @@ locate_and_pad_parm (passed_mode, type, 
     = type ? size_in_bytes (type) : size_int (GET_MODE_SIZE (passed_mode));
   enum direction where_pad = FUNCTION_ARG_PADDING (passed_mode, type);
   int boundary = FUNCTION_ARG_BOUNDARY (passed_mode, type);
+#ifdef ARGS_GROW_DOWNWARD
+  tree s2 = sizetree;
+#endif
 
 #ifdef REG_PARM_STACK_SPACE
   /* If we have found a stack parm before we reach the end of the
@@ -5289,13 +5290,20 @@ locate_and_pad_parm (passed_mode, type, 
       offset_ptr->constant = -initial_offset_ptr->constant;
       offset_ptr->var = 0;
     }
+
   if (where_pad != none
       && (!host_integerp (sizetree, 1)
 	  || (tree_low_cst (sizetree, 1) * BITS_PER_UNIT) % PARM_BOUNDARY))
-    sizetree = round_up (sizetree, PARM_BOUNDARY / BITS_PER_UNIT);
-  SUB_PARM_SIZE (*offset_ptr, sizetree);
-  if (where_pad != downward)
+    s2 = round_up (s2, PARM_BOUNDARY / BITS_PER_UNIT);
+  SUB_PARM_SIZE (*offset_ptr, s2);
+
+  if (!in_regs
+#ifdef REG_PARM_STACK_SPACE
+      || REG_PARM_STACK_SPACE (fndecl) > 0
+#endif
+     )
     pad_to_arg_alignment (offset_ptr, boundary, alignment_pad);
+
   if (initial_offset_ptr->var)
     arg_size_ptr->var = size_binop (MINUS_EXPR,
 				    size_binop (MINUS_EXPR,
@@ -5307,6 +5315,14 @@ locate_and_pad_parm (passed_mode, type, 
     arg_size_ptr->constant = (-initial_offset_ptr->constant
 			      - offset_ptr->constant);
 
+  /* Pad_below needs the pre-rounded size to know how much to pad below.
+     We only pad BLKmode parameters which are not in registers.  BLKmode
+     parameters passed in registers have their padding done elsewhere.  */
+  if (where_pad == downward
+      && passed_mode == BLKmode
+      && !in_regs)
+    pad_below (offset_ptr, passed_mode, sizetree);
+
 #else /* !ARGS_GROW_DOWNWARD */
   if (!in_regs
 #ifdef REG_PARM_STACK_SPACE
@@ -5392,7 +5408,6 @@ pad_to_arg_alignment (offset_ptr, bounda
     }
 }
 
-#ifndef ARGS_GROW_DOWNWARD
 static void
 pad_below (offset_ptr, passed_mode, sizetree)
      struct args_size *offset_ptr;
@@ -5420,7 +5435,6 @@ pad_below (offset_ptr, passed_mode, size
 	}
     }
 }
-#endif
 \f
 /* Walk the tree of blocks describing the binding levels within a function
    and warn about uninitialized variables.
Index: config/pa/pa-64.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa-64.h,v
retrieving revision 1.11
diff -u -3 -p -r1.11 pa-64.h
--- config/pa/pa-64.h	4 Sep 2002 18:09:32 -0000	1.11
+++ config/pa/pa-64.h	10 Sep 2002 16:36:45 -0000
@@ -88,8 +88,26 @@ Boston, MA 02111-1307, USA.  */
 #undef STATIC_CHAIN_REGNUM
 #define STATIC_CHAIN_REGNUM 31
 
-/* Nonzero if we do not know how to pass TYPE solely in registers.  */
-#define MUST_PASS_IN_STACK(MODE,TYPE) \
-  ((TYPE) != 0							\
-   && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST		\
-       || TREE_ADDRESSABLE (TYPE)))
+/* Define to 1 on a big-endian system to pass structures in a little-endian
+   manner.  The HP ABI specifies that structures and other aggregates are to
+   be left-justified (i.e., padded on the right).  The default padding
+   for big-endian is to right-justify structures that have a size smaller
+   than PARM_BOUNDARY and to left-justify larger ones.  Now, the order
+   of the HP argument registers are reversed, so we need to use a PARALLEL
+   to load the registers in the correct order.  This changes the default
+   justification to right-justication because emit_group_{load,store} are
+   now used.  So, we define FUNCTION_ARG_REG_LITTLE_ENDIAN to reverse this
+   behavior and get back to left-justification.  However, we don't use
+   a PARALLEL for structures smaller than PARM_BOUNDARY since this results
+   in incorrect justification.  For these, we return a REG.  Yikes!  */
+
+#define FUNCTION_ARG_REG_LITTLE_ENDIAN 1
+
+/* If defined, a C expression which determines whether the default
+   implementation of va_arg will attempt to pad down before reading the
+   next argument, if that argument is smaller than its aligned space as
+   controlled by PARM_BOUNDARY.  If this macro is not defined, all such
+   arguments are padded down when BYTES_BIG_ENDIAN is true.  We don't
+   want aggregrates padded down.  */
+
+#define PAD_VARARGS_DOWN (!AGGREGATE_TYPE_P (type))
Index: config/pa/pa.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa.c,v
retrieving revision 1.179
diff -u -3 -p -r1.179 pa.c
--- config/pa/pa.c	4 Sep 2002 18:09:32 -0000	1.179
+++ config/pa/pa.c	10 Sep 2002 16:36:46 -0000
@@ -5089,22 +5089,26 @@ function_arg_padding (mode, type)
      enum machine_mode mode;
      tree type;
 {
-  int size;
-
   if (mode == BLKmode)
     {
-      if (type && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST)
-	size = int_size_in_bytes (type) * BITS_PER_UNIT;
+      /* Return none if left or right justification is not required.  */
+      if (type
+	  && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
+	  && (int_size_in_bytes (type) * BITS_PER_UNIT) % PARM_BOUNDARY == 0)
+	return none;
+
+      if (TARGET_64BIT)
+	/* The 64-bit runtime specifies left justification.  */
+        return upward;
       else
-	return upward;		/* Don't know if this is right, but */
-				/* same as old definition.  */
+	/* The 32-bit runtime architecture specifies right justification.
+	   This is highly non standard for blocks larger than one word and
+	   needs special handling.  */
+	return downward;
     }
-  else
-    size = GET_MODE_BITSIZE (mode);
-  if (size < PARM_BOUNDARY)
+
+  if (GET_MODE_BITSIZE (mode) < PARM_BOUNDARY)
     return downward;
-  else if (size % PARM_BOUNDARY)
-    return upward;
   else
     return none;
 }
@@ -5201,8 +5205,11 @@ hppa_va_arg (valist, type)
 
   if (TARGET_64BIT)
     {
-      /* Every argument in PA64 is passed by value (including large structs).
-         Arguments with size greater than 8 must be aligned 0 MOD 16.  */
+      /* Every argument in PA64 is supposed to be passed by value (including
+	 large structs).  However, as a GCC extension, we pass zero and
+	 variable sized types by reference.
+
+	 Arguments with a size greater than 8 must be aligned 0 MOD 16.  */
 
       size = int_size_in_bytes (type);
       if (size > UNITS_PER_WORD)
@@ -5215,7 +5222,21 @@ hppa_va_arg (valist, type)
           TREE_SIDE_EFFECTS (t) = 1;
           expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
         }
-      return std_expand_builtin_va_arg (valist, type);
+      else if (size <= 0)
+	{
+	  t = build (PREDECREMENT_EXPR, TREE_TYPE (valist), valist,
+		     build_int_2 (POINTER_SIZE / BITS_PER_UNIT, 0));
+	  TREE_SIDE_EFFECTS (t) = 1;
+
+	  pptr = build_pointer_type (ptr);
+	  t = build1 (NOP_EXPR, pptr, t);
+	  TREE_SIDE_EFFECTS (t) = 1;
+
+	  t = build1 (INDIRECT_REF, ptr, t);
+	  TREE_SIDE_EFFECTS (t) = 1;
+	}
+      else
+	return std_expand_builtin_va_arg (valist, type);
     }
 
   /* Compute the rounded size of the type.  */
@@ -5224,8 +5245,8 @@ hppa_va_arg (valist, type)
 
   ptr = build_pointer_type (type);
 
-  /* "Large" types are passed by reference.  */
-  if (size > 8)
+  /* "Large" and variable sized types are passed by reference.  */
+  if (size > 8 || size <= 0)
     {
       t = build (PREDECREMENT_EXPR, TREE_TYPE (valist), valist,
 		 build_int_2 (POINTER_SIZE / BITS_PER_UNIT, 0));
@@ -5263,7 +5284,7 @@ hppa_va_arg (valist, type)
     }
 
   /* Calculate!  */
-  return expand_expr (t, NULL_RTX, Pmode, EXPAND_NORMAL);
+  return expand_expr (t, NULL_RTX, VOIDmode, EXPAND_NORMAL);
 }
 
 
@@ -7446,28 +7467,37 @@ function_arg (cum, mode, type, named, in
      int incoming;
 {
   int max_arg_words = (TARGET_64BIT ? 8 : 4);
-  int arg_size = FUNCTION_ARG_SIZE (mode, type);
   int alignment = 0;
+  int arg_size;
   int fpr_reg_base;
   int gpr_reg_base;
   rtx retval;
 
+  if (mode == VOIDmode)
+    return NULL_RTX;
+
+  arg_size = FUNCTION_ARG_SIZE (mode, type);
+
+  /* This might happen if someone tries to pass a variable sized structure
+     or union.  */
+  if (arg_size == 0)
+    abort ();
+
+  /* If this arg would be passed partially or totally on the stack, then
+     this routine should return zero.  FUNCTION_ARG_PARTIAL_NREGS will
+     handle arguments which are split between regs and stack slots if
+     the ABI mandates split arguments.  */
   if (! TARGET_64BIT)
     {
-      /* If this arg would be passed partially or totally on the stack, then
-         this routine should return zero.  FUNCTION_ARG_PARTIAL_NREGS will
-         handle arguments which are split between regs and stack slots if
-         the ABI mandates split arguments.  */
-      if (cum->words + arg_size > max_arg_words
-          || mode == VOIDmode)
+      /* The 32-bit ABI does not split arguments.  */
+      if (cum->words + arg_size > max_arg_words)
 	return NULL_RTX;
     }
   else
     {
       if (arg_size > 1)
 	alignment = cum->words & 1;
-      if (cum->words + alignment >= max_arg_words
-	  || mode == VOIDmode)
+      if (cum->words + alignment >= max_arg_words)
 	return NULL_RTX;
     }
 
@@ -7533,6 +7563,22 @@ function_arg (cum, mode, type, named, in
 	    {
 	      gpr_reg_base = 25;
 	      fpr_reg_base = 34;
+	    }
+
+	  /* Structures 5 to 8 bytes in size are passed in the general
+	     registers in the same manner as other non floating-point
+	     objects.  The data is right-justified and zero-extended
+	     to 64 bits.  We use a PARALLEL instead of a REG to obtain
+	     the correct justifiction.  PARALLELs use emit_group_{load,
+	     store} and these functions right-justify for big-endian.  */
+	  if (mode == BLKmode)
+	    {
+	      rtx loc[1];
+
+	      loc[0] = gen_rtx_EXPR_LIST (VOIDmode,
+					  gen_rtx_REG (DImode, gpr_reg_base),
+					  const0_rtx);
+	      return gen_rtx_PARALLEL (mode, gen_rtvec_v (1, loc));
 	    }
 	}
       else
Index: config/pa/pa.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa.h,v
retrieving revision 1.166
diff -u -3 -p -r1.166 pa.h
--- config/pa/pa.h	21 Aug 2002 02:41:50 -0000	1.166
+++ config/pa/pa.h	10 Sep 2002 16:36:46 -0000
@@ -407,7 +407,7 @@ extern int target_flags;
 
 /* Largest alignment required for any stack parameter, in bits.
    Don't define this if it is equal to PARM_BOUNDARY */
-#define MAX_PARM_BOUNDARY 64
+#define MAX_PARM_BOUNDARY (2 * PARM_BOUNDARY)
 
 /* Boundary (in *bits*) on which stack pointer is always aligned;
    certain optimizations in combine depend on this.
@@ -506,9 +506,14 @@ extern struct rtx_def *hppa_pic_save_rtx
    PA64 ABI says that objects larger than 128 bits are returned in memory.
    Note, int_size_in_bytes can return -1 if the size of the object is
    variable or larger than the maximum value that can be expressed as
-   a HOST_WIDE_INT.  */
+   a HOST_WIDE_INT.   It can also return zero for an empty type.  The
+   simplest way to handle variable and empty types is to pass them in
+   memory.  This avoids problems in defining the boundaries of argument
+   slots, allocating registers, etc.  */
 #define RETURN_IN_MEMORY(TYPE)	\
-  ((unsigned HOST_WIDE_INT) int_size_in_bytes (TYPE) > (TARGET_64BIT ? 16 : 8))
+  ((TYPE) == 0							\
+   || int_size_in_bytes (TYPE) > (TARGET_64BIT ? 16 : 8)	\
+   || int_size_in_bytes (TYPE) <= 0)
 
 /* Register in which address to store a structure value
    is passed to a function.  */
@@ -681,16 +686,18 @@ extern struct rtx_def *hppa_pic_save_rtx
    otherwise, FUNC is 0.  */
 
 /* On the HP-PA the value is found in register(s) 28(-29), unless
-   the mode is SF or DF. Then the value is returned in fr4 (32, ) */
+   the mode is SF or DF. Then the value is returned in fr4 (32).  */
 
 /* This must perform the same promotions as PROMOTE_MODE, else
    PROMOTE_FUNCTION_RETURN will not work correctly.  */
-#define FUNCTION_VALUE(VALTYPE, FUNC)				\
-  gen_rtx_REG (((INTEGRAL_TYPE_P (VALTYPE)			\
-		 && TYPE_PRECISION (VALTYPE) < BITS_PER_WORD)	\
-		|| POINTER_TYPE_P (VALTYPE))			\
-	       ? word_mode : TYPE_MODE (VALTYPE),		\
-	       TREE_CODE (VALTYPE) == REAL_TYPE && !TARGET_SOFT_FLOAT ? 32 : 28)
+#define FUNCTION_VALUE(VALTYPE, FUNC)					\
+  gen_rtx_REG (((INTEGRAL_TYPE_P (VALTYPE)				\
+		 && TYPE_PRECISION (VALTYPE) < BITS_PER_WORD)		\
+		|| POINTER_TYPE_P (VALTYPE))				\
+	        ? word_mode : TYPE_MODE (VALTYPE),			\
+	       (TREE_CODE (VALTYPE) == REAL_TYPE			\
+		&& TYPE_MODE (VALTYPE) != TFmode			\
+		&& !TARGET_SOFT_FLOAT) ? 32 : 28)
 
 /* Define how to find the value returned by a library function
    assuming the value has mode MODE.  */
@@ -745,7 +752,9 @@ struct hppa_args {int words, nargs_proto
   (CUM).indirect = 0,				\
   (CUM).nargs_prototype = 1000
 
-/* Figure out the size in words of the function argument.  */
+/* Figure out the size in words of the function argument.  The size
+   returned by this macro should always be greater than zero because
+   we pass variable and zero sized objects by reference.  */
 
 #define FUNCTION_ARG_SIZE(MODE, TYPE)	\
   ((((MODE) != BLKmode \
@@ -817,6 +826,17 @@ struct hppa_args {int words, nargs_proto
 #define FUNCTION_ARG(CUM, MODE, TYPE, NAMED) \
   function_arg (&CUM, MODE, TYPE, NAMED, 0)
 
+/* This needs to be set to force structure arguments with a single
+   field to be treated as structures and not as the type of their
+   field.  */
+#define MEMBER_TYPE_FORCES_BLK(FIELD, MODE) 1
+
+/* Nonzero if we do not know how to pass TYPE solely in registers.  */
+#define MUST_PASS_IN_STACK(MODE,TYPE) \
+  ((TYPE) != 0							\
+   && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST		\
+       || TREE_ADDRESSABLE (TYPE)))
+
 #define FUNCTION_INCOMING_ARG(CUM, MODE, TYPE, NAMED) \
   function_arg (&CUM, MODE, TYPE, NAMED, 1)
 
@@ -833,33 +853,37 @@ struct hppa_args {int words, nargs_proto
    bits, of an argument with the specified mode and type.  If it is
    not defined,  `PARM_BOUNDARY' is used for all arguments.  */
 
-#define FUNCTION_ARG_BOUNDARY(MODE, TYPE)			\
-  (((TYPE) != 0)						\
-   ? ((integer_zerop (TYPE_SIZE (TYPE))				\
-       || ! TREE_CONSTANT (TYPE_SIZE (TYPE)))			\
-      ? BITS_PER_UNIT						\
-      : (((int_size_in_bytes (TYPE)) + UNITS_PER_WORD - 1)	\
-	 / UNITS_PER_WORD) * BITS_PER_WORD)			\
-   : ((GET_MODE_ALIGNMENT(MODE) <= PARM_BOUNDARY)		\
-      ? PARM_BOUNDARY : GET_MODE_ALIGNMENT(MODE)))
-
-/* Arguments larger than eight bytes are passed by invisible reference */
+/* Arguments larger than one word are double word aligned.  */
 
-/* PA64 does not pass anything by invisible reference.  */
+#define FUNCTION_ARG_BOUNDARY(MODE, TYPE)				\
+  (((TYPE)								\
+    ? (integer_zerop (TYPE_SIZE (TYPE))					\
+       || !TREE_CONSTANT (TYPE_SIZE (TYPE))				\
+       || int_size_in_bytes (TYPE) <= UNITS_PER_WORD)			\
+    : GET_MODE_SIZE(MODE) <= UNITS_PER_WORD)				\
+   ? PARM_BOUNDARY : MAX_PARM_BOUNDARY)
+
+/* In the 32-bit runtime, arguments larger than eight bytes are passed
+   by invisible reference.  As a GCC extension, we also pass anything
+   with a zero or variable size by reference.
+
+   The 64-bit runtime does not describe passing any types by invisible
+   reference.  The internals of GCC can't currently handle passing
+   empty structures, and zero or variable length arrays when they are
+   not passed entirely on the stack or by reference.  Thus, as a GCC
+   extension, we pass these types by reference.  The HP compiler doesn't
+   support these types, so hopefully there shouldn't be any compatibility
+   issues.  This may have to be revisited when HP releases a C99 compiler
+   or updates the ABI.  */
 #define FUNCTION_ARG_PASS_BY_REFERENCE(CUM, MODE, TYPE, NAMED)		\
   (TARGET_64BIT								\
-   ? 0									\
-   : (((TYPE) && int_size_in_bytes (TYPE) > 8)				\
+   ? ((TYPE) && int_size_in_bytes (TYPE) <= 0)				\
+   : (((TYPE) && (int_size_in_bytes (TYPE) > 8				\
+		  || int_size_in_bytes (TYPE) <= 0))			\
       || ((MODE) && GET_MODE_SIZE (MODE) > 8)))
  
-/* PA64 does not pass anything by invisible reference.
-   This should be undef'ed for 64bit, but we'll see if this works. The
-   problem is that we can't test TARGET_64BIT from the preprocessor.  */
-#define FUNCTION_ARG_CALLEE_COPIES(CUM, MODE, TYPE, NAMED) \
-  (TARGET_64BIT							\
-   ? 0								\
-   : (((TYPE) && int_size_in_bytes (TYPE) > 8)			\
-      || ((MODE) && GET_MODE_SIZE (MODE) > 8)))
+#define FUNCTION_ARG_CALLEE_COPIES(CUM, MODE, TYPE, NAMED) 		\
+  FUNCTION_ARG_PASS_BY_REFERENCE (CUM, MODE, TYPE, NAMED)
 
 \f
 extern GTY(()) rtx hppa_compare_op0;
Index: testsuite/gcc.dg/struct-ret-1.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/testsuite/gcc.dg/struct-ret-1.c,v
retrieving revision 1.2
diff -u -3 -p -r1.2 struct-ret-1.c
--- testsuite/gcc.dg/struct-ret-1.c	16 Dec 1998 22:23:30 -0000	1.2
+++ testsuite/gcc.dg/struct-ret-1.c	10 Sep 2002 16:36:46 -0000
@@ -1,5 +1,6 @@
 /* { dg-do run { target hppa*-*-* } } */
 /* { dg-options { -O2 } { target hppa*-*-* } } */
+extern void exit (int);
 typedef struct {
         int             x;
         int             y;
@@ -16,7 +17,7 @@ main(int argc, char *argv[])
         if (printPoints(toPoint(0, 0), toPoint(1000, 1000)) != 1)
                 abort();
         else
-                exit();
+                exit(0);
 
         return 0;
 }

Re: Combine deletes non noop insns

[Richard Henderson]

On Tue, Sep 10, 2002 at 01:26:41PM -0400, John David Anglin wrote:
> (insn 42 41 43 0 (nil) (set (zero_extract:SI (subreg:SI (reg:QI 125) 0)
>             (const_int 8 [0x8])
>             (const_int 24 [0x18]))
>         (reg:SI 97)) 220 {insv_32} (insn_list 4 (insn_list 41 (nil)))

This seems odd to me.  Inserting data into the ignored 
portion of a paradoxical subreg?


r~

Re: Combine deletes non noop insns

[John David Anglin]

> On Tue, Sep 10, 2002 at 01:26:41PM -0400, John David Anglin wrote:
> > (insn 42 41 43 0 (nil) (set (zero_extract:SI (subreg:SI (reg:QI 125) 0)
> >             (const_int 8 [0x8])
> >             (const_int 24 [0x18]))
> >         (reg:SI 97)) 220 {insv_32} (insn_list 4 (insn_list 41 (nil)))
> 
> This seems odd to me.  Inserting data into the ignored 
> portion of a paradoxical subreg?

The data is inserted to the correct place but I see that the
subreg number is wrong.  It's probably another problem
related to the right justification of small BLKmode types.  How
the code that generated this managed to get the insert location
and the number bits to insert correct, but not the subreg is
strange.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: Combine deletes non noop insns

[Jeff Law]

In message <20020910184157.GB18949@redhat.com>, Richard Henderson writes:
 >On Tue, Sep 10, 2002 at 01:26:41PM -0400, John David Anglin wrote:
 >> (insn 42 41 43 0 (nil) (set (zero_extract:SI (subreg:SI (reg:QI 125) 0)
 >>             (const_int 8 [0x8])
 >>             (const_int 24 [0x18]))
 >>         (reg:SI 97)) 220 {insv_32} (insn_list 4 (insn_list 41 (nil)))
 >
 >This seems odd to me.  Inserting data into the ignored 
 >portion of a paradoxical subreg?
Regardless, we shouldn't have zapped the following store (insn 43),
at least that's the way it looks from the information John posted.
jeff

Re: Combine deletes non noop insns

[David Edelsohn]

>>>>> John David Anglin writes:

John> The data is inserted to the correct place but I see that the
John> subreg number is wrong.  It's probably another problem
John> related to the right justification of small BLKmode types.  How
John> the code that generated this managed to get the insert location
John> and the number bits to insert correct, but not the subreg is
John> strange.

	I'd start looking at rtlanal.c:subreg_regno_offset() which Joern
added to fix a similar problem last July.

David

Re: Combine deletes non noop insns

[John David Anglin]

> >>>>> John David Anglin writes:
> 
> John> The data is inserted to the correct place but I see that the
> John> subreg number is wrong.  It's probably another problem
> John> related to the right justification of small BLKmode types.  How
> John> the code that generated this managed to get the insert location
> John> and the number bits to insert correct, but not the subreg is
> John> strange.
> 
> 	I'd start looking at rtlanal.c:subreg_regno_offset() which Joern
> added to fix a similar problem last July.

It looks like it comes during the initial rtl generation:

#0  gen_rtx_fmt_eee (code=143, mode=SImode, arg0=0x4019db00, arg1=0x40018250,
    arg2=0x349e04) at genrtl.c:581
#1  0x0016e7c0 in gen_insv_32 (operand0=0x8f, operand1=0x4,
    operand2=0x4019db00, operand3=0x4019daa0) at insn-emit.c:1207
#2  0x001740f8 in gen_insv (operand0=0x40018250, operand1=0x40018250,
    operand2=0x400182d0, operand3=0x4019daa0) at insn-emit.c:5409
#3  0x000dde2c in store_bit_field (str_rtx=0x349e04, bitsize=8, bitnum=0,
    fieldmode=SImode, value=0x4019daa0, total_size=5)
    at ../../gcc/gcc/expmed.c:673
#4  0x000de140 in store_bit_field (str_rtx=0x349e04, bitsize=8, bitnum=0,
    fieldmode=SImode, value=0x4019daa0, total_size=5)
    at ../../gcc/gcc/expmed.c:599
#5  0x000de294 in store_bit_field (str_rtx=0x349e04, bitsize=40, bitnum=0,
    fieldmode=DImode, value=0x4019d1e0, total_size=5)
    at ../../gcc/gcc/expmed.c:487
#6  0x000e9158 in emit_group_store (orig_dst=0x4019d1d0, src=0x4018bbf0,
    ssize=5) at ../../gcc/gcc/expr.c:2416
#7  0x001411b0 in assign_parms (fndecl=0x401a2230)
    at ../../gcc/gcc/function.c:4690

(gdb) p debug_rtx (arg0)
(subreg:SI (reg:QI 125) 0)
(gdb) frame 2
#2  0x001740f8 in gen_insv (operand0=0x40018250, operand1=0x40018250,
    operand2=0x400182d0, operand3=0x4019daa0) at insn-emit.c:5409
5409        emit_insn (gen_insv_32 (operands[0], operands[1],
(gdb) p debug_rtx (operand0)
(const_int 8 [0x8])
$8 = void
(gdb) p debug_rtx (operand1)
(const_int 8 [0x8])
$9 = void
(gdb) p debug_rtx (operand2)
(const_int 24 [0x18])
$10 = void
(gdb) p debug_rtx (operand3)
(subreg:SI (reg:DI 94) 0)

I think I see the problem.  It appears to be in the insv expander.
Joern fixed a similar problem recently in extv expander.

Thanks,
Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: Combine deletes non noop insns

[Richard Henderson]

On Tue, Sep 10, 2002 at 01:18:17PM -0600, Jeff Law wrote:
>  >This seems odd to me.  Inserting data into the ignored 
>  >portion of a paradoxical subreg?
> Regardless, we shouldn't have zapped the following store (insn 43),
> at least that's the way it looks from the information John posted.

Yes, we could, if combine proves that reg 125 is unchanged
from when it was loaded.


r~

Re: Combine deletes non noop insns

[John David Anglin]

> On Tue, Sep 10, 2002 at 01:18:17PM -0600, Jeff Law wrote:
> >  >This seems odd to me.  Inserting data into the ignored 
> >  >portion of a paradoxical subreg?
> > Regardless, we shouldn't have zapped the following store (insn 43),
> > at least that's the way it looks from the information John posted.
> 
> Yes, we could, if combine proves that reg 125 is unchanged
> from when it was loaded.

Problem fixed.  Tested on hppa-linux and applied to main.  The patch
is similar to one applied by Joern on July 22.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

2002-09-10  John David Anglin  <dave@hiauly1.hia.nrc.ca>

	* pa.md (extzv): Check predicates before emitting extzv_32.
	(insv): Likewise.

Index: config/pa/pa.md
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa.md,v
retrieving revision 1.112
diff -u -3 -p -r1.112 pa.md
--- config/pa/pa.md	9 Sep 2002 21:34:48 -0000	1.112
+++ config/pa/pa.md	10 Sep 2002 22:42:07 -0000
@@ -6621,8 +6621,13 @@
     emit_insn (gen_extzv_64 (operands[0], operands[1],
 			     operands[2], operands[3]));
   else
-    emit_insn (gen_extzv_32 (operands[0], operands[1],
-			     operands[2], operands[3]));
+    {
+      if (! uint5_operand (operands[2], SImode)
+	  || ! uint5_operand (operands[3], SImode))
+	FAIL;
+      emit_insn (gen_extzv_32 (operands[0], operands[1],
+			       operands[2], operands[3]));
+    }
   DONE;
 }")
 
@@ -6741,8 +6746,13 @@
     emit_insn (gen_insv_64 (operands[0], operands[1],
 			    operands[2], operands[3]));
   else
-    emit_insn (gen_insv_32 (operands[0], operands[1],
-			    operands[2], operands[3]));
+    {
+      if (! uint5_operand (operands[2], SImode)
+	  || ! uint5_operand (operands[3], SImode))
+	FAIL;
+      emit_insn (gen_insv_32 (operands[0], operands[1],
+			      operands[2], operands[3]));
+    }
   DONE;
 }")
 

Re: Combine deletes non noop insns

[Jeff Law]

In message <20020911000346.GC19055@redhat.com>, Richard Henderson writes:
 >On Tue, Sep 10, 2002 at 01:18:17PM -0600, Jeff Law wrote:
 >>  >This seems odd to me.  Inserting data into the ignored 
 >>  >portion of a paradoxical subreg?
 >> Regardless, we shouldn't have zapped the following store (insn 43),
 >> at least that's the way it looks from the information John posted.
 >
 >Yes, we could, if combine proves that reg 125 is unchanged
 >from when it was loaded.
Good point.
jeff