[Bug target/27842] New: Miscompile of Altivec vec_abs (float) inside loop

[Bug target/27842] New: Miscompile of Altivec vec_abs (float) inside loop

[uweigand at gcc dot gnu dot org]

The following test case gets miscompiled and fails when built
with "-O -maltivec -mabi=altivec -include altivec.h" on GCC 4.1:

extern void abort (void);

void test (vector float *p, int n)
{
  int i;
  for (i = 0; i < n; i++)
    p[i] = vec_abs (p[i]);
}

int
main (void)
{
  vector float p = (vector float){ 0.5, 0.5, 0.5, 0.5 };
  vector float q = p;

  test (&p, 1);

  if (memcmp (&p, &q, sizeof (p)))
    abort ();

  return 0;
}


The reason for this appears to be an abuse of RTL semantics by
the "altivec_vspltisw_v4sf" pattern:

(define_insn "altivec_vspltisw_v4sf"
  [(set (match_operand:V4SF 0 "register_operand" "=v")
        (vec_duplicate:V4SF
         (float:SF (match_operand:QI 1 "s5bit_cint_operand" "i"))))]
  "TARGET_ALTIVEC"
  "vspltisw %0,%1"
  [(set_attr "type" "vecperm")])

What this instruction does is to load an immediate *integer*
value into a vector register, which happens to be re-interpreted
as a floating point value (without changing the bit pattern).

What the RTL associated with the pattern *says*, however, is
to load the integer, *converted to floating point*, into the
register, which is a quite different semantics.

Now, since the pattern is only explicitly generated from within
other expanders inside altivec.md (apparently), all of which
expect the semantics of the actual Altivec instruction, not the
semantics as literally specified in the RTL, those misinterpretations
generally cancel each other and generated code behaves as expected.

However, as soon as the middle-end gets an opportunity to run the
RTL through the simplifier, everything breaks.  This happens in
particular when the load is being hoisted out of a loop due to
being loop-invariant, as in the above test case: the vec_abs
pattern expands via this expander

;; Generate
;;    vspltisw SCRATCH1,-1
;;    vslw SCRATCH2,SCRATCH1,SCRATCH1
;;    vandc %0,%1,SCRATCH2
(define_expand "absv4sf2"
  [(set (match_dup 2)
        (vec_duplicate:V4SF (float:SF (const_int -1))))
   (set (match_dup 3)
        (unspec:V4SF [(match_dup 2) (match_dup 2)] UNSPEC_VSLW))
   (set (match_operand:V4SF 0 "register_operand" "=v")
        (and:V4SF (not:V4SF (match_dup 3))
                  (match_operand:V4SF 1 "register_operand" "v")))]
  "TARGET_ALTIVEC"
{
  operands[2] = gen_reg_rtx (V4SFmode);
  operands[3] = gen_reg_rtx (V4SFmode);
})

and the first two insns are in fact loop invariant.

The problem in this particular test case is a regression in GCC 4.1,
introduced by this patch:

[PATCH] Improve scheduling of Altivec absolute value patterns
http://gcc.gnu.org/ml/gcc-patches/2005-02/msg01195.html

-(define_insn "absv4sf2"
-  [(set (match_operand:V4SF 0 "register_operand" "=v")
-        (abs:V4SF (match_operand:V4SF 1 "register_operand" "v")))
-   (clobber (match_scratch:V4SF 2 "=&v"))
-   (clobber (match_scratch:V4SF 3 "=&v"))]
+;; Generate
+;;    vspltisw SCRATCH1,-1
+;;    vslw SCRATCH2,SCRATCH1,SCRATCH1
+;;    vandc %0,%1,SCRATCH2
+(define_expand "absv4sf2"
+  [(set (match_dup 2)
+       (vec_duplicate:V4SF (float:SF (const_int -1))))
+   (set (match_dup 3)
+        (unspec:V4SF [(match_dup 2) (match_dup 2)] UNSPEC_VSLW))
+   (set (match_operand:V4SF 0 "register_operand" "=v")
+        (and:V4SF (not:V4SF (match_dup 3))
+                  (match_operand:V4SF 1 "register_operand" "v")))]
   "TARGET_ALTIVEC"
-  "vspltisw %2,-1\;vslw %3,%2,%2\;vandc %0,%1,%3"
-  [(set_attr "type" "vecsimple")
-   (set_attr "length" "12")])
+{
+  operands[2] = gen_reg_rtx (V4SFmode);
+  operands[3] = gen_reg_rtx (V4SFmode);
+})

However, the underlying abuse of RTL semantics when describing the
vspltisw instruction in V4SFmode apparently pre-dates this patch.


The easiest way to fix this would appear to use an UNSPEC to
describe the insn semantics.  Any better idea?


-- 
           Summary: Miscompile of Altivec vec_abs (float) inside loop
           Product: gcc
           Version: 4.1.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: uweigand at gcc dot gnu dot org
GCC target triplet: powerpc*-*-*


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2006-05-31 16:50 -------
Maybe:


(define_insn "altivec_vspltisw_v4sf"
  [(set (match_operand:V4SF 0 "register_operand" "=v")
        (vec_duplicate:V4SF
         (subreg:SF (match_operand:QI 1 "s5bit_cint_operand" "i"))))]

OR something like that, even though it is not a subregister, it should still
work I think ?


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[uweigand at gcc dot gnu dot org]

------- Comment #2 from uweigand at gcc dot gnu dot org  2006-05-31 16:59 -------
I'm not sure (subreg:SF (const_int)) is canonical RTL, I haven't seen
subregs of anything but REG or MEM.

In any case, I don't really see what this would buy us over an UNSPEC -- will
the  generic simplifier be able to evaluate this by re-interpreting the bit
pattern
as float according to the target representation?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[pinskia at gcc dot gnu dot org]

------- Comment #3 from pinskia at gcc dot gnu dot org  2006-05-31 17:13 -------
What about this, deleting altivec_vspltisw_v4sf and changing all references to
gen_altivec_vspltisw_v4sf 
to something like
gen_altivec_vspltisw( gen_lowpart  (V4SImode, reg), ...);

That should keep the same code and correct the problem (and should help out
with some code gen in some cases).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[uweigand at gcc dot gnu dot org]

------- Comment #4 from uweigand at gcc dot gnu dot org  2006-06-01 21:30 -------
Yes, that makes sense -- in fact, it looks like altivec_vslw_v4sf can then be
removed as well.  I'm currenly testing a patch to that effect ...


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[pinskia at gcc dot gnu dot org]

------- Comment #5 from pinskia at gcc dot gnu dot org  2006-06-01 21:31 -------
Thanks for taking care of this.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-06-01 21:31:41
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[patchapp at dberlin dot org]

------- Comment #6 from patchapp at dberlin dot org  2006-06-02 16:00 -------
Subject: Bug number PR target/27842

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is
http://gcc.gnu.org/ml/gcc-patches/2006-06/msg00078.html


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[uweigand at gcc dot gnu dot org]

------- Comment #7 from uweigand at gcc dot gnu dot org  2006-06-06 17:01 -------
Subject: Bug 27842

Author: uweigand
Date: Tue Jun  6 17:01:27 2006
New Revision: 114438

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=114438
Log:
        PR target/27842
        * config/rs6000/altivec.md (UNSPEC_VSLW): Remove.
        ("altivec_vspltisw_v4sf", "altivec_vslw_v4sf"): Remove.
        ("mulv4sf3", "absv4sf3", "negv4sf3"): Adapt users to use
        V4SImode temporaries and operations instead.

        PR target/27842
        * gcc.dg/vmx/pr27842.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/vmx/pr27842.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/altivec.md
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[uweigand at gcc dot gnu dot org]

------- Comment #8 from uweigand at gcc dot gnu dot org  2006-06-06 17:05 -------
Subject: Bug 27842

Author: uweigand
Date: Tue Jun  6 17:04:56 2006
New Revision: 114439

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=114439
Log:
        PR target/27842
        * config/rs6000/altivec.md (UNSPEC_VSLW): Remove.
        ("altivec_vspltisw_v4sf", "altivec_vslw_v4sf"): Remove.
        ("mulv4sf3", "absv4sf3", "negv4sf3"): Adapt users to use
        V4SImode temporaries and operations instead.

        PR target/27842
        * gcc.dg/vmx/pr27842.c: New test.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/vmx/pr27842.c
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/config/rs6000/altivec.md
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842

[Bug target/27842] Miscompile of Altivec vec_abs (float) inside loop

[uweigand at gcc dot gnu dot org]

------- Comment #9 from uweigand at gcc dot gnu dot org  2006-06-06 17:10 -------
Fixed on 4.1 branch and mainline.


-- 

uweigand at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=27842