[Bug rtl-optimization/64286] Redundant extend removal ignores vector element type

["jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Igor Zamyatin from comment #1)
> Perhaps something like below to restrict ree for such cases?
> 
> diff --git a/gcc/ree.c b/gcc/ree.c
> index 3376901..92370ea 100644
> --- a/gcc/ree.c
> +++ b/gcc/ree.c
> @@ -1004,6 +1004,11 @@ add_removable_extension (const_rtx expr, rtx_insn
> *insn,
>        struct df_link *defs, *def;
>        ext_cand *cand;
>  
> +      if (!SCALAR_INT_MODE_P (GET_MODE (dest))
> +	  && (GET_MODE_UNIT_PRECISION (mode) !=
> +	      GET_MODE_UNIT_PRECISION (GET_MODE (XEXP (src, 0)))))
> +	return;
> +
>        /* First, make sure we can get all the reaching definitions.  */
>        defs = get_defs (insn, XEXP (src, 0), NULL);
>        if (!defs)

I think your patch is too restrictive.
Consider -O2 -mavx2:
typedef char __v16qi __attribute__((__vector_size__(16)));
typedef int __m128i __attribute__((__vector_size__(16)));
__m128i bar (__m128i);
typedef int __m256i __attribute__((__vector_size__(32)));
__m256i v;

void
foo (char *p)
{
  __m128i a = (__m128i)__builtin_ia32_loaddqu (p);
  __m128i ps1 = bar (a);
  v = (__m256i) __builtin_ia32_pmovzxbw256 ((__v16qi) a);
}

Here, there is:
(insn 19 9 11 2 (set (reg:V16QI 22 xmm1 [92])
        (mem/c:V16QI (plus:DI (reg/f:DI 6 bp)
                (const_int -32 [0xffffffffffffffe0])) [2 %sfp+-16 S16 A128]))
pr64286.i:12 1185 {*movv16qi_internal}
     (nil))
(insn 11 19 13 2 (set (reg:V16HI 22 xmm1 [orig:93 D.2299 ] [93])
        (zero_extend:V16HI (reg:V16QI 22 xmm1 [92]))) pr64286.i:12 3826
{avx2_zero_extendv16qiv16hi2}
     (nil))
and there is no reason to restrict it.  I also don't understand the
GET_MODE_UNIT_PRECISION != GET_MODE_UNIT_PRECISION test, do you know about
SIGN_EXTEND/ZERO_EXTEND where the unit precision is the same?  That wouldn't be
an extension.
The important difference between vectors and scalars is that for scalars the
lowpart subreg of the zero/sign extended value is still the original value,
while for vectors that is not the case.  So, for vectors you can REE optimize
them only if all the uses are the same extension (zero vs. sign, and to the
same mode).

Therefore, supposedly for non-scalar modes (i.e. vector ones, other than scalar
int and vector int hopefully don't have zero/sign_extend) I think what should
be done is bail out if any of the defs has any uses that are not the sign resp.
zero extension that has been found.
We have there the:

      /* Second, make sure the reaching definitions don't feed another and
         different extension.  FIXME: this obviously can be improved.  */
      for (def = defs; def; def = def->next)
        if ((idx = def_map[INSN_UID (DF_REF_INSN (def->ref))])
            && (cand = &(*insn_list)[idx - 1])
            && cand->code != code)
          {
            if (dump_file)
              {
                fprintf (dump_file, "Cannot eliminate extension:\n");
                print_rtl_single (dump_file, insn);
                fprintf (dump_file, " because of other extension\n");
              }
            return;
          }

loop, perhaps for the vector modes we could add
else if (!SCALAR_INT_MODE_P (...) && idx == 0)
and in that case look using DU chains (which are supposedly computed) if any
uses of it other than the current insn are not a sign/zero extension at all or
are different extension or to different mode than the current instruction, and
in that case record some magic value to def_map (e.g -1U) and treat later that
magic def_map value as a sign that we should give up (disregard that
extension).
>From gcc-bugs-return-472630-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jan 09 16:22:02 2015
Return-Path: <gcc-bugs-return-472630-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 5431 invoked by alias); 9 Jan 2015 16:22:00 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 5400 invoked by uid 48); 9 Jan 2015 16:21:55 -0000
From: "belagod at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug testsuite/63971] Some of  gcc.target/aarch64/test_frame_*.c tests fail now
Date: Fri, 09 Jan 2015 16:22:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: testsuite
X-Bugzilla-Version: 5.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: belagod at gcc dot gnu.org
X-Bugzilla-Status: REOPENED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: pinskia at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status cc resolution
Message-ID: <bug-63971-4-X2tNtnXtN1@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-63971-4@http.gcc.gnu.org/bugzilla/>
References: <bug-63971-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-01/txt/msg00624.txt.bz2
Content-length: 534

https://gcc.gnu.org/bugzilla/show_bug.cgi?idc971

Tejas Belagod <belagod at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |belagod at gcc dot gnu.org
         Resolution|FIXED                       |---

--- Comment #4 from Tejas Belagod <belagod at gcc dot gnu.org> ---
It looks like this still fails on trunk.