Re: split_complex_* flaw

[Alan Modra <amodra at bigpond dot net dot au>]

On Sun, Feb 01, 2004 at 01:48:47AM -0500, David Edelsohn wrote:
> >>>>> Alan Modra writes:
> 
> Alan> This is where things go wrong.  va_list on powerpc64-linux is just
> Alan> a simple pointer.  va_arg(ap, _Complex float) is looking for a pair
> Alan> of adjacent floats in a doubleword, but as you say, the stack image
> Alan> is two doublewords with the split_complex_* code.
> 
> 	Either the default implementation of GCC's std_builtin_va_arg
> machinery needs to do something special for SPLIT_COMPLEX_ARGS or each
> port needs to do something special.

I don't like the idea very much.  We have this nice simple va_list
type, and now _Complex float is going to be an odd case.  Why should
_Complex float be treated differently?  How do we explain that passing
struct { _Complex float } as a variable arg can't be accessed as
a _Complex float, or vice versa?  Does the split_complex code handle
those anonymous union thingies?  What else is going to bite us later?

I'd much rather dispense with the split_complex_* code and fix the
backend.  It's not so hard to handle complex types.  In fact, I'm
bootstrapping a patch (third iteration or so..) for the hammer branch
now, and results look promising.  The major piece, in function_arg,
looks like:

       if (USE_FP_FOR_ARG_P (cum, mode, type))
 	{
-	  if (! type
-	      || ((cum->nargs_prototype > 0)
-	          /* IBM AIX extended its linkage convention definition always
-		     to require FP args after register save area hole on the
-		     stack.  */
-	          && (DEFAULT_ABI != ABI_AIX
-		      || ! TARGET_XL_CALL
-		      || (align_words < GP_ARG_NUM_REG))))
-	    return gen_rtx_REG (mode, cum->fregno);
+	  rtx fpr[5];
+	  rtx *r;
+	  bool needs_psave;
+	  unsigned long n = 0;
+	  unsigned long n_fpreg = rs6000_num_fpreg (mode);
+	  enum machine_mode fmode = mode;
+	  unsigned long i;
+	  unsigned long off;
+
+	  if (cum->fregno + n_fpreg > FP_ARG_MAX_REG + 1)
+	    {
+	      /* Long double or complex split over regs and memory.
+		 Describe fpreg part. */
+	      if (GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT)
+		fmode = GET_MODE_INNER (mode);
+	      if (fmode == TFmode)
+		fmode = DFmode;
+	      do
+		fpr[n + 1] = gen_rtx_REG (fmode, cum->fregno + n);
+	      while (++n <= FP_ARG_MAX_REG - cum->fregno);
+	    }
+	  else if (GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT
+		   && RS6000_SPLIT_COMPLEX_ARGS)
+	    {
+	      fmode = GET_MODE_INNER (mode);
+	      if (fmode == TFmode)
+		fmode = DFmode;
+	      do
+		fpr[n + 1] = gen_rtx_REG (fmode, cum->fregno + n);
+	      while (++n < n_fpreg);
+	    }
+	  else
+	    fpr[++n] = gen_rtx_REG (mode, cum->fregno);
+
+	  /* Do we also need to pass this arg in the parameter save
+	     area?  */
+	  needs_psave = (type
+			 && (cum->nargs_prototype <= 0
+			     || (DEFAULT_ABI == ABI_AIX
+				 && TARGET_XL_CALL
+				 && align_words >= GP_ARG_NUM_REG)));
+
+	  if (!needs_psave && mode == fmode)
+	    return fpr[1];
+
+	  /* Describe where all the pieces go.  */
+	  r = fpr + 1;
+	  for (i = 0, off = 0; i < n; i++, off += GET_MODE_SIZE (fmode))
+	    r[i] = gen_rtx_EXPR_LIST (VOIDmode, r[i], GEN_INT (off));
+
+	  if (needs_psave)
+	    {
+	      /* Now describe the part that goes in gprs or the stack.
+		 This piece must come first, before the fprs.  */
+	      rtx reg = NULL_RTX;
+	      if (align_words < GP_ARG_NUM_REG)
+		{
+		  unsigned long n_words = rs6000_arg_size (mode, type);
+		  enum machine_mode rmode = mode;
+
+		  if (align_words + n_words > GP_ARG_NUM_REG)
+		    /* If this is partially on the stack, then we only
+		       include the portion actually in registers here.  */
+		    rmode = Pmode;
+		  reg = gen_rtx_REG (rmode, GP_ARG_MIN_REG + align_words);
+		}
+	      *--r = gen_rtx_EXPR_LIST (VOIDmode, reg, const0_rtx);
+	      ++n;
+	    }
 
-          return gen_rtx_PARALLEL (mode,
-	    gen_rtvec (2,
-		       gen_rtx_EXPR_LIST (VOIDmode,
-				((align_words >= GP_ARG_NUM_REG)
-				 ? NULL_RTX
-				 : (align_words
-				    + RS6000_ARG_SIZE (mode, type)
-				    > GP_ARG_NUM_REG
-				    /* If this is partially on the stack, then
-				       we only include the portion actually
-				       in registers here.  */
-				    ? gen_rtx_REG (SImode,
-					       GP_ARG_MIN_REG + align_words)
-				    : gen_rtx_REG (mode,
-					       GP_ARG_MIN_REG + align_words))),
-				const0_rtx),
-		       gen_rtx_EXPR_LIST (VOIDmode,
-				gen_rtx_REG (mode, cum->fregno),
-				const0_rtx)));
+          return gen_rtx_PARALLEL (mode, gen_rtvec_v (n, r));
 	}
       else if (align_words < GP_ARG_NUM_REG)
 	return gen_rtx_REG (mode, GP_ARG_MIN_REG + align_words);


-- 
Alan Modra
IBM OzLabs - Linux Technology Centre