public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Ulrich Weigand" <uweigand@de.ibm.com>
To: ebotcazou@adacore.com (Eric Botcazou)
Cc: iant@google.com, gcc-patches@gcc.gnu.org,
	        trevor_smigiel@playstation.sony.com,
	        andrew_pinski@playstation.sony.com
Subject: [PATCH, v2] Re: [PATCH] Fix ICE in simplify_immed_subreg on SPU
Date: Thu, 26 Jun 2008 22:15:00 -0000	[thread overview]
Message-ID: <200806262209.m5QM9Ww5024891@d12av02.megacenter.de.ibm.com> (raw)
In-Reply-To: <200806160027.42235.ebotcazou@adacore.com> from "Eric Botcazou" at Jun 16, 2008 12:27:42 AM

Eric Botcazou wrote:

> > I think I'm with Ulrich here.  A CONST_VECTOR containing a CONST seems
> > reasonable to me.  Eric, can you clarify what you see as the problem?
> 
> I don't see the point in changing the semantics of CONST_VECTOR because of
> a IMO non-representative testcase.

I've looked at this again in more detail, and it seems that there is
indeed the pervasive assumption that CONST_VECTOR elements can only
be CONST_INT, CONST_DOUBLE, and CONST_FIXED.  For example, dwarf2out.c
cannot handle anything else when constructing an DW_AT_const_value
attribute refering to a CONST_VECTOR.

While I still think it would be better to allow any legitimate constant
as member of a CONST_VECTOR, this would require non-trivial changes to
dwarf2out.c and a couple of other places.  Therefore, for now I think
it is best to change the SPU back-end to avoid generating CONST_VECTOR
expressions that are invalid according to the current middle-end rules.
At some later time those middle-end rules could be relaxed ...

The following (SPU back-end only) patch implements this.  While this
might result in somewhat less efficient code being generated for
certain case, those cases should be rather rare.

Trevor/Andrew, what are your thoughts about this?  Would this be OK
for mainline and 4.3?

Tested on spu-elf with no regressions, fixes the pr34856 FAILs.

Bye,
Ulrich


ChangeLog:

	PR target/34856
	* config/spu/spu.c (spu_builtin_splats): Do not generate
	invalid CONST_VECTOR expressions.
	(spu_expand_vector_init): Likewise.


Index: gcc/config/spu/spu.c
===================================================================
*** gcc/config/spu/spu.c	(revision 136912)
--- gcc/config/spu/spu.c	(working copy)
*************** spu_builtin_splats (rtx ops[])
*** 4576,4590 ****
        constant_to_array (GET_MODE_INNER (mode), ops[1], arr);
        emit_move_insn (ops[0], array_to_constant (mode, arr));
      }
-   else if (!flag_pic && GET_MODE (ops[0]) == V4SImode && CONSTANT_P (ops[1]))
-     {
-       rtvec v = rtvec_alloc (4);
-       RTVEC_ELT (v, 0) = ops[1];
-       RTVEC_ELT (v, 1) = ops[1];
-       RTVEC_ELT (v, 2) = ops[1];
-       RTVEC_ELT (v, 3) = ops[1];
-       emit_move_insn (ops[0], gen_rtx_CONST_VECTOR (mode, v));
-     }
    else
      {
        rtx reg = gen_reg_rtx (TImode);
--- 4576,4581 ----
*************** spu_expand_vector_init (rtx target, rtx 
*** 4903,4909 ****
    for (i = 0; i < n_elts; ++i)
      {
        x = XVECEXP (vals, 0, i);
!       if (!CONSTANT_P (x))
  	++n_var;
        else
  	{
--- 4894,4902 ----
    for (i = 0; i < n_elts; ++i)
      {
        x = XVECEXP (vals, 0, i);
!       if (!(CONST_INT_P (x)
! 	    || GET_CODE (x) == CONST_DOUBLE
! 	    || GET_CODE (x) == CONST_FIXED))
  	++n_var;
        else
  	{
*************** spu_expand_vector_init (rtx target, rtx 
*** 4940,4947 ****
  	  /* fill empty slots with the first constant, this increases
  	     our chance of using splats in the recursive call below. */
  	  for (i = 0; i < n_elts; ++i)
! 	    if (!CONSTANT_P (XVECEXP (constant_parts_rtx, 0, i)))
! 	      XVECEXP (constant_parts_rtx, 0, i) = first_constant;
  
  	  spu_expand_vector_init (target, constant_parts_rtx);
  	}
--- 4933,4945 ----
  	  /* fill empty slots with the first constant, this increases
  	     our chance of using splats in the recursive call below. */
  	  for (i = 0; i < n_elts; ++i)
! 	    {
! 	      x = XVECEXP (constant_parts_rtx, 0, i);
! 	      if (!(CONST_INT_P (x)
! 		    || GET_CODE (x) == CONST_DOUBLE
! 		    || GET_CODE (x) == CONST_FIXED))
! 		XVECEXP (constant_parts_rtx, 0, i) = first_constant;
! 	    }
  
  	  spu_expand_vector_init (target, constant_parts_rtx);
  	}
*************** spu_expand_vector_init (rtx target, rtx 
*** 4957,4963 ****
        for (i = 0; i < n_elts; ++i)
  	{
  	  x = XVECEXP (vals, 0, i);
! 	  if (!CONSTANT_P (x))
  	    {
  	      if (!register_operand (x, GET_MODE (x)))
  		x = force_reg (GET_MODE (x), x);
--- 4955,4963 ----
        for (i = 0; i < n_elts; ++i)
  	{
  	  x = XVECEXP (vals, 0, i);
! 	  if (!(CONST_INT_P (x)
! 		|| GET_CODE (x) == CONST_DOUBLE
! 		|| GET_CODE (x) == CONST_FIXED))
  	    {
  	      if (!register_operand (x, GET_MODE (x)))
  		x = force_reg (GET_MODE (x), x);

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

  parent reply	other threads:[~2008-06-26 22:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-15 15:53 Ulrich Weigand
2008-06-15 17:27 ` Eric Botcazou
2008-06-15 17:43   ` Ulrich Weigand
2008-06-15 22:24     ` Eric Botcazou
2008-06-15 22:25     ` Ian Lance Taylor
2008-06-15 22:45       ` Eric Botcazou
2008-06-15 22:49         ` Andrew Pinski
2008-06-26 22:15         ` Ulrich Weigand [this message]
2008-06-26 23:14           ` [PATCH, v2] " trevor_smigiel
2008-06-27  7:05           ` Eric Botcazou
2008-06-28 17:53             ` Ulrich Weigand
2008-06-30  8:27               ` Eric Botcazou

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=200806262209.m5QM9Ww5024891@d12av02.megacenter.de.ibm.com \
    --to=uweigand@de.ibm.com \
    --cc=andrew_pinski@playstation.sony.com \
    --cc=ebotcazou@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=iant@google.com \
    --cc=trevor_smigiel@playstation.sony.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).