public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Li, Pan2" <pan2.li@intel.com>
To: Hongtao Liu <crazylht@gmail.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>,
	"kito.cheng@gmail.com" <kito.cheng@gmail.com>,
	"richard.guenther@gmail.com" <richard.guenther@gmail.com>,
	"Wang, Yanzhang" <yanzhang.wang@intel.com>,
	"rdapp.gcc@gmail.com" <rdapp.gcc@gmail.com>
Subject: RE: [PATCH v1] RTL: Bugfix ICE after allow vector type in DSE
Date: Mon, 26 Feb 2024 03:42:42 +0000	[thread overview]
Message-ID: <MW5PR11MB59089C90556702435AFF748DA95A2@MW5PR11MB5908.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CAMZc-bz8oLPHRXnGHJ69v9D5kSOLp5he2-iTxAiQTZg5V9_gnQ@mail.gmail.com>

> Be Careful, It may regresses some other backend.

Thanks Hongtao, how about take INNER_MODE here for regsize. Currently it will be the whole vector register when comparation.

poly_uint64 regsize = REGMODE_NATURAL_SIZE (imode);

Pan

-----Original Message-----
From: Hongtao Liu <crazylht@gmail.com> 
Sent: Monday, February 26, 2024 11:41 AM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; richard.guenther@gmail.com; Wang, Yanzhang <yanzhang.wang@intel.com>; rdapp.gcc@gmail.com
Subject: Re: [PATCH v1] RTL: Bugfix ICE after allow vector type in DSE

On Mon, Feb 26, 2024 at 11:26 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> We allowed vector type for get_stored_val when read is less than or
> equal to store in previous.  Unfortunately, we missed to adjust the
> validate_subreg part accordingly.  For vector type, we don't need to
> restrict the mode size is greater than the vector register size.
>
> Thus, for example when gen_lowpart from E_V2SFmode to E_V4QImode, it
> will have NULL_RTX(of course ICE after that) because of the mode size
> is less than vector register size.  That also explain that gen_lowpart
> from E_V8SFmode to E_V16QImode is valid here.
>
> This patch would like to remove the the restriction for vector mode, to
> rid of the ICE when gen_lowpart because of validate_subreg fails.
Be Careful, It may regresses some other backend.
>
> The below test are passed for this patch:
>
> * The X86 bootstrap test.
> * The fully riscv regression tests.
>
> gcc/ChangeLog:
>
>         * emit-rtl.cc (validate_subreg): Bypass register size check
>         if the mode is vector.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/ssa-fre-44.c: Add ftree-vectorize to trigger
>         the ICE.
>         * gcc.target/riscv/rvv/base/bug-6.c: New test.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/emit-rtl.cc                               |  3 ++-
>  gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c    |  2 +-
>  .../gcc.target/riscv/rvv/base/bug-6.c         | 22 +++++++++++++++++++
>  3 files changed, 25 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
>
> diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
> index 1856fa4884f..45c6301b487 100644
> --- a/gcc/emit-rtl.cc
> +++ b/gcc/emit-rtl.cc
> @@ -934,7 +934,8 @@ validate_subreg (machine_mode omode, machine_mode imode,
>      ;
>    /* ??? Similarly, e.g. with (subreg:DF (reg:TI)).  Though store_bit_field
>       is the culprit here, and not the backends.  */
> -  else if (known_ge (osize, regsize) && known_ge (isize, osize))
> +  else if (known_ge (isize, osize) && (known_ge (osize, regsize)
> +    || (VECTOR_MODE_P (imode) || VECTOR_MODE_P (omode))))
>      ;
>    /* Allow component subregs of complex and vector.  Though given the below
>       extraction rules, it's not always clear what that means.  */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c
> index f79b4c142ae..624a00a4f32 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O -fdump-tree-fre1" } */
> +/* { dg-options "-O -fdump-tree-fre1 -O3 -ftree-vectorize" } */
>
>  struct A { float x, y; };
>  struct B { struct A u; };
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
> new file mode 100644
> index 00000000000..5bb00b8f587
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
> @@ -0,0 +1,22 @@
> +/* Test that we do not have ice when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -ftree-vectorize" } */
> +
> +struct A { float x, y; };
> +struct B { struct A u; };
> +
> +extern void bar (struct A *);
> +
> +float
> +f3 (struct B *x, int y)
> +{
> +  struct A p = {1.0f, 2.0f};
> +  struct A *q = &x[y].u;
> +
> +  __builtin_memcpy (&q->x, &p.x, sizeof (float));
> +  __builtin_memcpy (&q->y, &p.y, sizeof (float));
> +
> +  bar (&p);
> +
> +  return x[y].u.x + x[y].u.y;
> +}
> --
> 2.34.1
>


-- 
BR,
Hongtao

  reply	other threads:[~2024-02-26  3:42 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-26  3:25 pan2.li
2024-02-26  3:41 ` Hongtao Liu
2024-02-26  3:42   ` Li, Pan2 [this message]
2024-02-26  5:13     ` Hongtao Liu
2024-02-26  7:38 ` Richard Biener
2024-02-26  7:41   ` Li, Pan2
2024-02-26 14:22 ` [PATCH v2] DSE: Bugfix ICE after allow vector type in get_stored_val pan2.li
2024-02-27  9:47   ` Richard Biener
2024-02-27 15:02   ` Jeff Law
2024-02-28  1:40     ` Li, Pan2
2024-02-28  4:51       ` Li, Pan2
2024-02-28 17:33         ` Jeff Law
2024-02-29  1:38           ` Li, Pan2
2024-02-29 13:28             ` Robin Dapp
2024-03-02  1:04               ` Li, Pan2
2024-03-03 22:46               ` Jeff Law
2024-03-05  6:22                 ` Li, Pan2
2024-03-12  2:08                   ` Li, Pan2
2024-03-22  1:15                     ` Li, Pan2
2024-03-22 18:53                   ` Jeff Law
2024-03-23  5:45                     ` Li, Pan2
2024-04-06 12:02                       ` Li, Pan2
2024-04-18  1:46                         ` Li, Pan2
2024-04-28 12:10                           ` Li, Pan2
2024-04-29 15:20                       ` Jeff Law
2024-04-30  1:02                         ` Li, Pan2
2024-04-30  7:17 ` [PATCH v3] DSE: Fix " pan2.li
2024-04-30 11:35   ` Li, Pan2
2024-05-03  1:57     ` Li, Pan2
2024-05-03  1:51 ` [PATCH v4] " pan2.li
2024-05-16  4:06   ` Li, Pan2
2024-05-19 16:23   ` Jeff Law
2024-05-20  1:08     ` Li, Pan2

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW5PR11MB59089C90556702435AFF748DA95A2@MW5PR11MB5908.namprd11.prod.outlook.com \
    --to=pan2.li@intel.com \
    --cc=crazylht@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=juzhe.zhong@rivai.ai \
    --cc=kito.cheng@gmail.com \
    --cc=rdapp.gcc@gmail.com \
    --cc=richard.guenther@gmail.com \
    --cc=yanzhang.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).