Patchwork [middle-end] : Fix PR 44569

login
register
mail settings
Submitter Uros Bizjak
Date Oct. 30, 2010, 2:22 p.m.
Message ID <AANLkTi=pLZxn0zP1U5k4OABuu=sKiFutw7DdacyOVdQs@mail.gmail.com>
Download mbox | patch
Permalink /patch/69656/
State New
Headers show

Comments

Uros Bizjak - Oct. 30, 2010, 2:22 p.m.
Hello!

For the testcase in PR [1] (and gcc.dg/torture/vector-shift2.c in a
duplicate [2]), gcc generates following debug statement:

(debug_insn 7 4 8 2 (var_location:V16QI vuchar (concatn:V16QI [
            (const_int 1 [0x1])
            (const_int 2 [0x2])
            (const_int 3 [0x3])
            (const_int 4 [0x4])
            (const_int 1 [0x1])
            (const_int 2 [0x2])
            (const_int 3 [0x3])
            (const_int 4 [0x4])
            (const_int 1 [0x1])
            (const_int 2 [0x2])
            (const_int 3 [0x3])
            (const_int 4 [0x4])
            (const_int 1 [0x1])
            (const_int 2 [0x2])
            (const_int 3 [0x3])
            (const_int 4 [0x4])
        ])) vector-shift2.c:14 -1
     (nil))

This RTX is processed in simplify_subreg_concatn, that breaks the RTX
into VOIDmode const_int elements and passes them to
simplify_gen_subreg. The later function can not handle VOIDmode
inner_mode argument.

So, in case of VOIDmode, just determine the mode of the elements by using
GET_MODE_INNER of vector mode of concatn RTX.

2010-10-30  Uros Bizjak  <ubizjak@gmail.com>

	PR middle-end/44569
	* lower-suberg.c (simplify_subreg_concatn): For VOIDmode elements,
	determine the mode of a subreg by GET_MODE_INNER of CONCATN RTX.

Patch was tested on x86_64-pc-linux-gnu {,-m32}, where it fixes
testcase from [1] and also gcc.dg/torture/vector-shift2.c failure from
[2].

OK for mainline, 4.4 and 4.5 ?

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44569
[2] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46227

Uros.
Richard Guenther - Oct. 30, 2010, 6:12 p.m.
On Sat, Oct 30, 2010 at 4:22 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
> For the testcase in PR [1] (and gcc.dg/torture/vector-shift2.c in a
> duplicate [2]), gcc generates following debug statement:
>
> (debug_insn 7 4 8 2 (var_location:V16QI vuchar (concatn:V16QI [
>            (const_int 1 [0x1])
>            (const_int 2 [0x2])
>            (const_int 3 [0x3])
>            (const_int 4 [0x4])
>            (const_int 1 [0x1])
>            (const_int 2 [0x2])
>            (const_int 3 [0x3])
>            (const_int 4 [0x4])
>            (const_int 1 [0x1])
>            (const_int 2 [0x2])
>            (const_int 3 [0x3])
>            (const_int 4 [0x4])
>            (const_int 1 [0x1])
>            (const_int 2 [0x2])
>            (const_int 3 [0x3])
>            (const_int 4 [0x4])
>        ])) vector-shift2.c:14 -1
>     (nil))
>
> This RTX is processed in simplify_subreg_concatn, that breaks the RTX
> into VOIDmode const_int elements and passes them to
> simplify_gen_subreg. The later function can not handle VOIDmode
> inner_mode argument.
>
> So, in case of VOIDmode, just determine the mode of the elements by using
> GET_MODE_INNER of vector mode of concatn RTX.
>
> 2010-10-30  Uros Bizjak  <ubizjak@gmail.com>
>
>        PR middle-end/44569
>        * lower-suberg.c (simplify_subreg_concatn): For VOIDmode elements,
>        determine the mode of a subreg by GET_MODE_INNER of CONCATN RTX.
>
> Patch was tested on x86_64-pc-linux-gnu {,-m32}, where it fixes
> testcase from [1] and also gcc.dg/torture/vector-shift2.c failure from
> [2].
>
> OK for mainline, 4.4 and 4.5 ?

Ok.

Thanks,
Richard.

> [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44569
> [2] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46227
>
> Uros.
>
Richard Sandiford - Nov. 2, 2010, 9:16 a.m.
Uros Bizjak <ubizjak@gmail.com> writes:
> For the testcase in PR [1] (and gcc.dg/torture/vector-shift2.c in a
> duplicate [2]), gcc generates following debug statement:
>
> (debug_insn 7 4 8 2 (var_location:V16QI vuchar (concatn:V16QI [
>             (const_int 1 [0x1])
>             (const_int 2 [0x2])
>             (const_int 3 [0x3])
>             (const_int 4 [0x4])
>             (const_int 1 [0x1])
>             (const_int 2 [0x2])
>             (const_int 3 [0x3])
>             (const_int 4 [0x4])
>             (const_int 1 [0x1])
>             (const_int 2 [0x2])
>             (const_int 3 [0x3])
>             (const_int 4 [0x4])
>             (const_int 1 [0x1])
>             (const_int 2 [0x2])
>             (const_int 3 [0x3])
>             (const_int 4 [0x4])
>         ])) vector-shift2.c:14 -1
>      (nil))

Out of interest, is this canonical rtl?  I wasn't sure whether it
should be a const_vector instead.

Richard
Richard Guenther - Nov. 2, 2010, 10:51 a.m.
On Tue, Nov 2, 2010 at 10:16 AM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> Uros Bizjak <ubizjak@gmail.com> writes:
>> For the testcase in PR [1] (and gcc.dg/torture/vector-shift2.c in a
>> duplicate [2]), gcc generates following debug statement:
>>
>> (debug_insn 7 4 8 2 (var_location:V16QI vuchar (concatn:V16QI [
>>             (const_int 1 [0x1])
>>             (const_int 2 [0x2])
>>             (const_int 3 [0x3])
>>             (const_int 4 [0x4])
>>             (const_int 1 [0x1])
>>             (const_int 2 [0x2])
>>             (const_int 3 [0x3])
>>             (const_int 4 [0x4])
>>             (const_int 1 [0x1])
>>             (const_int 2 [0x2])
>>             (const_int 3 [0x3])
>>             (const_int 4 [0x4])
>>             (const_int 1 [0x1])
>>             (const_int 2 [0x2])
>>             (const_int 3 [0x3])
>>             (const_int 4 [0x4])
>>         ])) vector-shift2.c:14 -1
>>      (nil))
>
> Out of interest, is this canonical rtl?  I wasn't sure whether it
> should be a const_vector instead.

We probably can end up with various non-canonical RTL in debug-insns
(like we can end up with non-gimple trees in debug statements).  Sad
but true.

Richard.

> Richard
>
Paolo Bonzini - Nov. 2, 2010, 6:31 p.m.
On 11/02/2010 11:51 AM, Richard Guenther wrote:
> >  Out of interest, is this canonical rtl?  I wasn't sure whether it
> >  should be a const_vector instead.
>
> We probably can end up with various non-canonical RTL in debug-insns
> (like we can end up with non-gimple trees in debug statements).  Sad
> but true.

If the choice is between a CONST_VECTOR and a CONCATN, a precise 
definition of the one that is incorrect (I don't know which) would be 
"wrong RTL" rather than "non-canonical RTL". :)

Paolo

Patch

Index: lower-subreg.c
===================================================================
--- lower-subreg.c	(revision 166086)
+++ lower-subreg.c	(working copy)
@@ -396,7 +396,7 @@  simplify_subreg_concatn (enum machine_mo
 			 unsigned int byte)
 {
   unsigned int inner_size;
-  enum machine_mode innermode;
+  enum machine_mode innermode, partmode;
   rtx part;
   unsigned int final_offset;
 
@@ -409,11 +409,19 @@  simplify_subreg_concatn (enum machine_mo
 
   inner_size = GET_MODE_SIZE (innermode) / XVECLEN (op, 0);
   part = XVECEXP (op, 0, byte / inner_size);
+  partmode = GET_MODE (part);
+
+  if (partmode == VOIDmode)
+    {
+      gcc_assert (VECTOR_MODE_P (innermode));
+      partmode = GET_MODE_INNER (innermode);
+    }
+
   final_offset = byte % inner_size;
   if (final_offset + GET_MODE_SIZE (outermode) > inner_size)
     return NULL_RTX;
 
-  return simplify_gen_subreg (outermode, part, GET_MODE (part), final_offset);
+  return simplify_gen_subreg (outermode, part, partmode, final_offset);
 }
 
 /* Wrapper around simplify_gen_subreg which handles CONCATN.  */