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]

[PATCH] PR target/10979: ICE with atan2 intrinsic on x86


Very many thanks to Andrew Pinski for bringing PR target/10979 to
my attention.  I'm still getting used to bugzilla, so whilst I've
been concentrating on middle-end bugs, this one I'd missed.

The problem is that i386.md's pattern for atan2df3 has an explicit
clobber of the second operand, a floating point register, as the
"FPATAN" instruction consumes this value and it isn't available for
later instructions.

The problem is that the middle end, places local floating point
variables into named pseudos, that expand_builtin_mathfn_2 then
passes directly to expand_binop, where gen_atan2df3 wraps one of
them in a clobber.  Any attempt to use that local after the call
to atan2 then results in bad things; life gets very confused and
reg-stack eventually dies.

The solution is simple, we copy the operands of binary builtin
math functions into new pseudos before calling expand_binop, which
is then free to destroy/clobber these values.  If an operand isn't
clobbered, CSE and/or register allocation will remove the redundant
register-register copy instruction.

Jan's not wrong when he says this part of the compiler is moving
pretty fast.  expand_builtin_mathfn_2 is modified almost daily :>

The following patch has been tested on i686-pc-linux-gnu with a
full "make -k bootstrap", all languages except treelang, and
regression tested with a top-level "make -k check" with no new
failures.


Ok for mainline?


2003-06-30  Roger Sayle  <roger@eyesopen.com>

	PR target/10979
	* builtins.c (expand_builtin_mathfn_2): Copy expanded arguments
	into fresh pseudos before calling expand_binop, as intrinsics
	may clobber their operands.  Remove now unused SUBTARGET argument.
	(expand_builtin_pow): Adjust call to expand_builtin_mathfn_2.
	(expand_builtin): Likewise.

	* gcc.dg/20030630-1.c: New testcase.


Index: builtins.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/builtins.c,v
retrieving revision 1.230
diff -c -3 -p -r1.230 builtins.c
*** builtins.c	29 Jun 2003 21:16:13 -0000	1.230
--- builtins.c	30 Jun 2003 01:40:03 -0000
*************** static enum type_class type_to_class (tr
*** 105,111 ****
  static rtx expand_builtin_classify_type (tree);
  static void expand_errno_check (tree, rtx);
  static rtx expand_builtin_mathfn (tree, rtx, rtx);
! static rtx expand_builtin_mathfn_2 (tree, rtx, rtx);
  static rtx expand_builtin_constant_p (tree, enum machine_mode);
  static rtx expand_builtin_args_info (tree);
  static rtx expand_builtin_next_arg (tree);
--- 105,111 ----
  static rtx expand_builtin_classify_type (tree);
  static void expand_errno_check (tree, rtx);
  static rtx expand_builtin_mathfn (tree, rtx, rtx);
! static rtx expand_builtin_mathfn_2 (tree, rtx);
  static rtx expand_builtin_constant_p (tree, enum machine_mode);
  static rtx expand_builtin_args_info (tree);
  static rtx expand_builtin_next_arg (tree);
*************** expand_builtin_mathfn (tree exp, rtx tar
*** 1840,1854 ****
  /* Expand a call to the builtin binary math functions (pow and atan2).
     Return 0 if a normal call should be emitted rather than expanding the
     function in-line.  EXP is the expression that is a call to the builtin
!    function; if convenient, the result should be placed in TARGET.
!    SUBTARGET may be used as the target for computing one of EXP's
!    operands.  */

  static rtx
! expand_builtin_mathfn_2 (tree exp, rtx target, rtx subtarget)
  {
    optab builtin_optab;
!   rtx op0, op1, insns;
    tree fndecl = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
    tree arglist = TREE_OPERAND (exp, 1);
    tree arg0, arg1, temp, narg;
--- 1840,1852 ----
  /* Expand a call to the builtin binary math functions (pow and atan2).
     Return 0 if a normal call should be emitted rather than expanding the
     function in-line.  EXP is the expression that is a call to the builtin
!    function; if convenient, the result should be placed in TARGET.  */

  static rtx
! expand_builtin_mathfn_2 (tree exp, rtx target)
  {
    optab builtin_optab;
!   rtx op0, op1, top0, top1, insns;
    tree fndecl = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
    tree arglist = TREE_OPERAND (exp, 1);
    tree arg0, arg1, temp, narg;
*************** expand_builtin_mathfn_2 (tree exp, rtx t
*** 1910,1924 ****
    if (! stable)
      exp = build_function_call_expr (fndecl, arglist);

!   op0 = expand_expr (arg0, subtarget, VOIDmode, 0);
!   op1 = expand_expr (arg1, 0, VOIDmode, 0);

    emit_queue ();
    start_sequence ();

    /* Compute into TARGET.
       Set TARGET to wherever the result comes back.  */
!   target = expand_binop (mode, builtin_optab, op0, op1,
  			 target, 0, OPTAB_DIRECT);

    /* If we were unable to expand via the builtin, stop the sequence
--- 1908,1930 ----
    if (! stable)
      exp = build_function_call_expr (fndecl, arglist);

!   /* Use new pseudos as intrinsics may clobber their operands.  */
!   top0 = gen_reg_rtx (mode);
!   op0 = expand_expr (arg0, top0, mode, 0);
!   if (op0 != top0)
!     emit_move_insn (top0, op0);
!
!   top1 = gen_reg_rtx (mode);
!   op1 = expand_expr (arg1, top1, mode, 0);
!   if (op1 != top1)
!     emit_move_insn (top1, op1);

    emit_queue ();
    start_sequence ();

    /* Compute into TARGET.
       Set TARGET to wherever the result comes back.  */
!   target = expand_binop (mode, builtin_optab, top0, top1,
  			 target, 0, OPTAB_DIRECT);

    /* If we were unable to expand via the builtin, stop the sequence
*************** expand_builtin_pow (tree exp, rtx target
*** 2185,2191 ****
            return expand_powi (op, mode, n);
  	}
      }
!   return expand_builtin_mathfn_2 (exp, target, NULL_RTX);
  }

  /* Expand expression EXP which is a call to the strlen builtin.  Return 0
--- 2191,2197 ----
            return expand_powi (op, mode, n);
  	}
      }
!   return expand_builtin_mathfn_2 (exp, target);
  }

  /* Expand expression EXP which is a call to the strlen builtin.  Return 0
*************** expand_builtin (tree exp, rtx target, rt
*** 4856,4862 ****
      case BUILT_IN_ATAN2L:
        if (! flag_unsafe_math_optimizations)
  	break;
!       target = expand_builtin_mathfn_2 (exp, target, subtarget);
        if (target)
  	return target;
        break;
--- 4862,4868 ----
      case BUILT_IN_ATAN2L:
        if (! flag_unsafe_math_optimizations)
  	break;
!       target = expand_builtin_mathfn_2 (exp, target);
        if (target)
  	return target;
        break;


/* Derived from PR target/10979.  */
/* This testcase used to ICE on x86.  */
/* { dg-do compile } */
/* { dg-options "-O2 -ffast-math" } */

void t(double);
double atan2(double,double);

void temp(double *c)
{
  double c2 = 8;
  double s2 = 0;
  *c = atan2(s2,c2);
  t(1/s2);
}


Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833


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