[Bug target/98799] New: [10 Regression] vector_set_var ICE

[Bug target/98799] New: [10 Regression] vector_set_var ICE

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98799

            Bug ID: 98799
           Summary: [10 Regression] vector_set_var ICE
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dje at gcc dot gnu.org
  Target Milestone: ---
            Target: powerpc-ibm-aix7.2.3.0

gcc.target/powerpc/pr79251.p8.c
gcc.target/powerpc/fold-vec-insert-float-p8.c

now ICE:

error: unrecognizable insn:
(insn 16 15 17 2 (parallel [
            (set (reg:SI 132)
                (unspec:SI [
                        (reg/v:SF 122 [ f ])
                    ] UNSPEC_SI_FROM_SF))
            (clobber (scratch:V4SF))
        ])
"/nasfarm/edelsohn/src/src/gcc/testsuite/gcc.target/powerpc/fold-vec-
insert-float-p8.c":13:10 -1
     (nil))
during RTL pass: vregs internal compiler error: in extract_insn, at
recog.c:2769
ranges offset out of range

[Bug target/98799] [10 Regression] vector_set_var ICE

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98799

David Edelsohn <dje at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |11.0
     Ever confirmed|0                           |1
           Priority|P3                          |P2
   Last reconfirmed|                            |2021-01-22
                 CC|                            |bergner at gcc dot gnu.org,
                   |                            |luoxhu at cn dot ibm.com,
                   |                            |segher at gcc dot gnu.org

--- Comment #1 from David Edelsohn <dje at gcc dot gnu.org> ---
Confirmed.

[Bug target/98799] [10 Regression] vector_set_var ICE

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98799

--- Comment #2 from David Edelsohn <dje at gcc dot gnu.org> ---
As with vector_extract, the P8 version requires TARGET_DIRECT_MOVE_64BIT.

I think that this needs something like the following:

--- rs6000.c    2021-01-22 11:45:39.000000000 -0500
+++ cc.c        2021-01-23 14:55:25.000000000 -0500
@@ -6993,9 +6993,14 @@ rs6000_expand_vector_set (rtx target, rt

   if (VECTOR_MEM_VSX_P (mode))
     {
-      if (!CONST_INT_P (elt_rtx))
+      if (!CONST_INT_P (elt_rtx)
+         && (TARGET_P9_VECTOR || TARGET_DIRECT_MOVE_64BIT))
        {
-         rs6000_expand_vector_set_var (target, val, elt_rtx);
+         if (TARGET_P9_VECTOR
+             || GET_MODE_SIZE (GET_MODE_INNER (GET_MODE (target))) == 8)
+           rs6000_expand_vector_set_var_p9 (target, val, idx);
+         else
+           rs6000_expand_vector_set_var_p8 (target, val, idx);
          return;
        }

@@ -7235,19 +7240,6 @@
     gen_vector_select_v16qi (target_v16qi, target_v16qi, val_perm,
mask_perm));
 }

-/* Insert VAL into IDX of TARGET, VAL size is same of the vector element, IDX
-   is variable and also counts by vector element size.  */
-
-void
-rs6000_expand_vector_set_var (rtx target, rtx val, rtx idx)
-{
-  machine_mode mode = GET_MODE (target);
-  machine_mode inner_mode = GET_MODE_INNER (mode);
-  if (TARGET_P9_VECTOR || GET_MODE_SIZE (inner_mode) == 8)
-    rs6000_expand_vector_set_var_p9 (target, val, idx);
-  else
-    rs6000_expand_vector_set_var_p8 (target, val, idx);
-}

 /* Extract field ELT from VEC into TARGET.  */


I also don't see the purpose of rs6000_expand_vector_set_var() helper function,
especially not as a global function.

[Bug target/98799] [10 Regression] vector_set_var ICE

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98799

--- Comment #3 from David Edelsohn <dje at gcc dot gnu.org> ---
Created attachment 50042
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50042&action=edit
patch

This is a version of the patch that compiles.  I moved the definitions of
vector_set_var after the p8 and p9 definitions instead of pre-declaring them.
They definitely should not have been declared in rs6000-protos.h.  And they
also should be declared static.

This patch fixes the ICE in extract_insn, but there still is a second failure
in the testcase itself.

[Bug target/98799] [10 Regression] vector_set_var ICE

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98799

David Edelsohn <dje at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50042|0                           |1
        is obsolete|                            |

--- Comment #4 from David Edelsohn <dje at gcc dot gnu.org> ---
Created attachment 50043
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50043&action=edit
patch

Updated patch, but the entire rs6000_expand_set_var() logic seems to be
incomplete and missing some scenarios, i.e., P9 and P8 that assume PPC64 are
not sufficient.

[Bug target/98799] [10 Regression] vector_set_var ICE

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98799

luoxhu at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |luoxhu at gcc dot gnu.org

--- Comment #5 from luoxhu at gcc dot gnu.org ---
(In reply to David Edelsohn from comment #4)
> Created attachment 50043 [details]
> patch
> 
> Updated patch, but the entire rs6000_expand_set_var() logic seems to be
> incomplete and missing some scenarios, i.e., P9 and P8 that assume PPC64 are
> not sufficient.

The ICE is caused by UNSPEC_SI_FROM_SF not supported when
TARGET_DIRECT_MOVE_64BIT
is false.
Thank for the patch, but also need below change to fix the ICE in
gcc.target/powerpc/fold-vec-insert-float-p8.c when build with -m32 to avoid
generate IFN VEC_SET for P8BE-32bit.
Not sure about the meaning of "P9 and P8 that assume PPC64 are not sufficient"?


diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index f6ee1e6..656cdb3 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -1600,7 +1600,7 @@ altivec_resolve_overloaded_builtin (location_t loc, tree
fndecl,
          stmt = build1 (COMPOUND_LITERAL_EXPR, arg1_type, stmt);
        }

-      if (TARGET_P8_VECTOR)
+      if (TARGET_P8_VECTOR && TARGET_DIRECT_MOVE_64BIT)
        {
          stmt = build_array_ref (loc, stmt, arg2);
          stmt = fold_build2 (MODIFY_EXPR, TREE_TYPE (arg0), stmt,

[Bug target/98799] [11 Regression] vector_set_var ICE

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98799

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Xiong Hu Luo <luoxhu@gcc.gnu.org>:

https://gcc.gnu.org/g:fbe37371cf372b84d5b7f1a6f5f0971a513dd5fa

commit r11-6947-gfbe37371cf372b84d5b7f1a6f5f0971a513dd5fa
Author: Xionghu Luo <luoxhu@linux.ibm.com>
Date:   Wed Jan 27 20:24:03 2021 -0600

    rs6000: Fix vec insert ilp32 ICE and test failures [PR98799]

    UNSPEC_SI_FROM_SF is not supported when TARGET_DIRECT_MOVE_64BIT
    is false for -m32, don't generate VIEW_CONVERT_EXPR(ARRAY_REF) for
    variable vector insert.  Remove rs6000_expand_vector_set_var helper
    function, adjust the p8 and p9 definitions position and make them
    static.

    The previous commit r11-6858 missed check m32, This patch is tested pass
    on P7BE{m32,m64}/P8BE{m32,m64}/P8LE/P9LE with
    RUNTESTFLAGS="--target_board =unix'{-m32,-m64}'" for BE targets.

    gcc/ChangeLog:

    2021-01-27  Xionghu Luo  <luoxhu@linux.ibm.com>
                David Edelsohn  <dje.gcc@gmail.com>

            PR target/98799
            * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin):
            Don't generate VIEW_CONVERT_EXPR for fcode
ALTIVEC_BUILTIN_VEC_INSERT
            when -m32.
            * config/rs6000/rs6000-protos.h (rs6000_expand_vector_set_var):
            Delete.
            * config/rs6000/rs6000.c (rs6000_expand_vector_set): Remove the
            wrapper call rs6000_expand_vector_set_var for cleanup.  Call
            rs6000_expand_vector_set_var_p9 and rs6000_expand_vector_set_var_p8
            directly.
            (rs6000_expand_vector_set_var): Delete.
            (rs6000_expand_vector_set_var_p9): Make static.
            (rs6000_expand_vector_set_var_p8): Make static.

    gcc/testsuite/ChangeLog:

    2021-01-27  Xionghu Luo  <luoxhu@linux.ibm.com>

            PR target/98827
            * gcc.target/powerpc/fold-vec-insert-char-p8.c: Adjust ilp32.
            * gcc.target/powerpc/fold-vec-insert-char-p9.c: Likewise.
            * gcc.target/powerpc/fold-vec-insert-double.c: Likewise.
            * gcc.target/powerpc/fold-vec-insert-float-p8.c: Likewise.
            * gcc.target/powerpc/fold-vec-insert-float-p9.c: Likewise.
            * gcc.target/powerpc/fold-vec-insert-int-p8.c: Likewise.
            * gcc.target/powerpc/fold-vec-insert-int-p9.c: Likewise.
            * gcc.target/powerpc/fold-vec-insert-longlong.c: Likewise.
            * gcc.target/powerpc/fold-vec-insert-short-p8.c: Likewise.
            * gcc.target/powerpc/fold-vec-insert-short-p9.c: Likewise.
            * gcc.target/powerpc/pr79251.p8.c: Likewise.
            * gcc.target/powerpc/pr79251.p9.c: Likewise.
            * gcc.target/powerpc/vsx-builtin-7.c: Likewise.
            * gcc.target/powerpc/pr79251-run.c: Build and run with vsx
            option.

[Bug target/98799] [11 Regression] vector_set_var ICE

[luoxhu at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98799

luoxhu at gcc dot gnu.org changed:

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

--- Comment #7 from luoxhu at gcc dot gnu.org ---
Fixed.