Patchwork [ia64] : Fix PR 51681, ICE in gcc.dg/torture/vshuf-v2si.c (+ more)

login
register
mail settings
Submitter Uros Bizjak
Date Jan. 2, 2012, 7:47 p.m.
Message ID <CAFULd4YLMkqhHroVogJbC3_GPdCKPxB3JkE2M8m37rB6-tJaaw@mail.gmail.com>
Download mbox | patch
Permalink /patch/133916/
State New
Headers show

Comments

Uros Bizjak - Jan. 2, 2012, 7:47 p.m.
Hello!

Attached patch fixes a couple of thinkos in new ia64 vector permutation code.

2012-01-02  Uros Bizjak  <ubizjak@gmail.com>

	PR target/51681
	* config/ia64/ia64.c (expand_vec_perm_shrp): Use correct operands
	for shrp pattern.  Correctly handle and fixup shift variable.

2012-01-02  Uros Bizjak  <ubizjak@gmail.com>

	* config/ia64/ia64.c (expand_vec_perm_broadcast): Use correct
	operands for extzv pattern.

Patch was bootstrapped and regression tested on
ia64-unknown-linux-gnu, and fixes following testsuite failures:

FAIL: gcc.dg/torture/vshuf-v2si.c  -O2  (internal compiler error)
FAIL: gcc.dg/torture/vshuf-v2si.c  -O2  (test for excess errors)
WARNING: gcc.dg/torture/vshuf-v2si.c  -O2  compilation failed to
produce executable
FAIL: gcc.dg/torture/vshuf-v8qi.c  -O2  execution test

OK for mainline?

Uros.
Richard Henderson - Jan. 2, 2012, 8:31 p.m.
On 01/03/2012 06:47 AM, Uros Bizjak wrote:
>    if (d->testing_p)
>      return true;
>  
> +  hi = shift < nelt ? d->op1 : d->op0;
> +  lo = shift < nelt ? d->op0 : d->op1;
> +
> +  shift %= nelt;

This bit only works for little-endian.  It's why I had that assert
for shift range 1-63, for one thing.

I guess an extra check for big-endian before the loop could fix that.
I.e.

   shift = d->perm[0];
+  if (BYTES_BIG_ENDIAN && shift > nelt)
+    return false;


r~
Uros Bizjak - Jan. 3, 2012, 10:50 a.m.
On Mon, Jan 2, 2012 at 9:31 PM, Richard Henderson <rth@redhat.com> wrote:
> On 01/03/2012 06:47 AM, Uros Bizjak wrote:
>>    if (d->testing_p)
>>      return true;
>>
>> +  hi = shift < nelt ? d->op1 : d->op0;
>> +  lo = shift < nelt ? d->op0 : d->op1;
>> +
>> +  shift %= nelt;
>
> This bit only works for little-endian.  It's why I had that assert
> for shift range 1-63, for one thing.
>
> I guess an extra check for big-endian before the loop could fix that.
> I.e.
>
>   shift = d->perm[0];
> +  if (BYTES_BIG_ENDIAN && shift > nelt)
> +    return false;

I tried to investigate -mbig-endian a bit, but unfortunately almost
all gcc.dg/torture/vshuf-*.c tests FAIL with -O2 on unpatched gcc.
Tests pass with -O0, though.

Due to this, I committed only the obvious part of the patch, that is:

2012-01-02  Uros Bizjak  <ubizjak@gmail.com>

       * config/ia64/ia64.c (expand_vec_perm_broadcast): Use correct
       operands for extzv pattern.

Uros.
Uros Bizjak - Jan. 3, 2012, 12:32 p.m.
On Tue, Jan 3, 2012 at 11:50 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Jan 2, 2012 at 9:31 PM, Richard Henderson <rth@redhat.com> wrote:
>> On 01/03/2012 06:47 AM, Uros Bizjak wrote:
>>>    if (d->testing_p)
>>>      return true;
>>>
>>> +  hi = shift < nelt ? d->op1 : d->op0;
>>> +  lo = shift < nelt ? d->op0 : d->op1;
>>> +
>>> +  shift %= nelt;
>>
>> This bit only works for little-endian.  It's why I had that assert
>> for shift range 1-63, for one thing.
>>
>> I guess an extra check for big-endian before the loop could fix that.
>> I.e.
>>
>>   shift = d->perm[0];
>> +  if (BYTES_BIG_ENDIAN && shift > nelt)
>> +    return false;
>
> I tried to investigate -mbig-endian a bit, but unfortunately almost
> all gcc.dg/torture/vshuf-*.c tests FAIL with -O2 on unpatched gcc.
> Tests pass with -O0, though.

ATM, we have gcc.dg/torture/vshuf*.c:

Native configuration is ia64-unknown-linux-gnu

                === gcc tests ===


Running target unix
FAIL: gcc.dg/torture/vshuf-v2si.c  -O2  (internal compiler error)
FAIL: gcc.dg/torture/vshuf-v2si.c  -O2  (test for excess errors)
UNRESOLVED: gcc.dg/torture/vshuf-v2si.c  -O2  compilation failed to
produce executable

                === gcc Summary for unix ===

# of expected passes            30
# of unexpected failures        2
# of unresolved testcases       1
# of unsupported tests          112

Running target unix/-mbig-endian
FAIL: gcc.dg/torture/vshuf-v16hi.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v16qi.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v2sf.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v2si.c  -O2  (internal compiler error)
FAIL: gcc.dg/torture/vshuf-v2si.c  -O2  (test for excess errors)
UNRESOLVED: gcc.dg/torture/vshuf-v2si.c  -O2  compilation failed to
produce executable
FAIL: gcc.dg/torture/vshuf-v32qi.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v4hi.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v4sf.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v4si.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v8hi.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v8qi.c  -O2  execution test
FAIL: gcc.dg/torture/vshuf-v8si.c  -O2  execution test

                === gcc Summary for unix/-mbig-endian ===

# of expected passes            20
# of unexpected failures        12
# of unresolved testcases       1
# of unsupported tests          112

                === gcc Summary ===

# of expected passes            50
# of unexpected failures        14
# of unresolved testcases       2
# of unsupported tests          224
/home/uros/gcc-build/gcc/xgcc  version 4.7.0 20120103 (experimental)
[trunk revision 182829] (GCC)

Uros.
Richard Henderson - Jan. 3, 2012, 8:36 p.m.
On 01/03/2012 09:50 PM, Uros Bizjak wrote:
> I tried to investigate -mbig-endian a bit, but unfortunately almost
> all gcc.dg/torture/vshuf-*.c tests FAIL with -O2 on unpatched gcc.
> Tests pass with -O0, though.

Tests without -O don't actually test this code.

And -mbig-endian doesn't get big-endian execution on linux, so it's
only useful for manually examining assembly and checking for ICEs.


r~

Patch

Index: config/ia64/ia64.c
===================================================================
--- config/ia64/ia64.c	(revision 182780)
+++ config/ia64/ia64.c	(working copy)
@@ -11085,7 +11085,7 @@  static bool
 expand_vec_perm_shrp (struct expand_vec_perm_d *d)
 {
   unsigned i, nelt = d->nelt, shift, mask;
-  rtx tmp, op0, op1;;
+  rtx tmp, hi, lo;
 
   /* ??? Don't force V2SFmode into the integer registers.  */
   if (d->vmode == V2SFmode)
@@ -11101,6 +11101,11 @@  expand_vec_perm_shrp (struct expand_vec_perm_d *d)
   if (d->testing_p)
     return true;
 
+  hi = shift < nelt ? d->op1 : d->op0;
+  lo = shift < nelt ? d->op0 : d->op1;
+
+  shift %= nelt;
+
   shift *= GET_MODE_UNIT_SIZE (d->vmode) * BITS_PER_UNIT;
 
   /* We've eliminated the shift 0 case via expand_vec_perm_identity.  */
@@ -11113,11 +11118,9 @@  expand_vec_perm_shrp (struct expand_vec_perm_d *d)
     shift = 64 - shift;
 
   tmp = gen_reg_rtx (DImode);
-  op0 = (shift < nelt ? d->op0 : d->op1);
-  op1 = (shift < nelt ? d->op1 : d->op0);
-  op0 = gen_lowpart (DImode, op0);
-  op1 = gen_lowpart (DImode, op1);
-  emit_insn (gen_shrp (tmp, op0, op1, GEN_INT (shift)));
+  hi = gen_lowpart (DImode, hi);
+  lo = gen_lowpart (DImode, lo);
+  emit_insn (gen_shrp (tmp, hi, lo, GEN_INT (shift)));
 
   emit_move_insn (d->target, gen_lowpart (d->vmode, tmp));
   return true;
@@ -11207,7 +11210,7 @@  expand_vec_perm_broadcast (struct expand_vec_perm_
       elt *= BITS_PER_UNIT;
       temp = gen_reg_rtx (DImode);
       emit_insn (gen_extzv (temp, gen_lowpart (DImode, d->op0),
-			    GEN_INT (elt), GEN_INT (8)));
+			    GEN_INT (8), GEN_INT (elt)));
       emit_insn (gen_mux1_brcst_qi (d->target, gen_lowpart (QImode, temp)));
       break;