From: <trevor_smigiel@playstation.sony.com>
To: Ulrich Weigand <uweigand@de.ibm.com>
Cc: Eric Botcazou <ebotcazou@adacore.com>,
iant@google.com, gcc-patches@gcc.gnu.org,
andrew_pinski@playstation.sony.com
Subject: Re: [PATCH, v2] Re: [PATCH] Fix ICE in simplify_immed_subreg on SPU
Date: Thu, 26 Jun 2008 23:14:00 -0000 [thread overview]
Message-ID: <20080626223710.GG30485@playstation.sony.com> (raw)
In-Reply-To: <200806262209.m5QM9Ww5024891@d12av02.megacenter.de.ibm.com>
Ulrich,
I have no additional information to refute the opinion that our usage is
"non-representative". We have code that uses these semantics, so it is
useful to us.
The patch is ok. Go ahead and check it in.
Trevor
* Ulrich Weigand <uweigand@de.ibm.com> [2008-06-26 15:15]:
> 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
next prev parent reply other threads:[~2008-06-26 22:38 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 ` [PATCH, v2] " Ulrich Weigand
2008-06-26 23:14 ` trevor_smigiel [this message]
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=20080626223710.GG30485@playstation.sony.com \
--to=trevor_smigiel@playstation.sony.com \
--cc=andrew_pinski@playstation.sony.com \
--cc=ebotcazou@adacore.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=iant@google.com \
--cc=uweigand@de.ibm.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).