diff mbox series

[i386] Fix ICE in pass_rpad.

Message ID 20210918030932.2907679-1-hongtao.liu@intel.com
State New
Headers
Series [i386] Fix ICE in pass_rpad. |

Commit Message

liuhongt Sept. 18, 2021, 3:09 a.m. UTC 
  Besides conversion instructions, pass_rpad also handles scalar
sqrt/rsqrt/rcp/round instructions, while r12-3614 should only want to
handle conversion instructions, so fix it.

  Bootstrapped and regtest on x86_64-linux-gnu{-m32,} w/ configure
--enable-checking=yes,rtl,extra, failed tests are fixed.
  Ok for trunk?

New tests that PASS (8 tests):

gcc.target/i386/avx512f-vscalefpd-2.c execution test
gcc.target/i386/avx512f-vscalefps-2.c execution test
gcc.target/i386/avx512f-vscalefss-2.c execution test
gcc.target/i386/avx512fp16-vscalefph-1b.c execution test
gcc.target/i386/avx512fp16-vscalefsh-1b.c execution test
gcc.target/i386/avx512fp16vl-vscalefph-1b.c execution test
gcc.target/i386/avx512vl-vscalefpd-2.c execution test
gcc.target/i386/avx512vl-vscalefps-2.c execution test

Old tests that failed, that have disappeared (8 tests): (Eeek!)

gcc.target/i386/avx512f-vscalefpd-2.c (internal compiler error)
gcc.target/i386/avx512f-vscalefps-2.c (internal compiler error)
gcc.target/i386/avx512f-vscalefss-2.c (internal compiler error)
gcc.target/i386/avx512fp16-vscalefph-1b.c (internal compiler error)
gcc.target/i386/avx512fp16-vscalefsh-1b.c (internal compiler error)
gcc.target/i386/avx512fp16vl-vscalefph-1b.c (internal compiler error)
gcc.target/i386/avx512vl-vscalefpd-2.c (internal compiler error)
gcc.target/i386/avx512vl-vscalefps-2.c (internal compiler error)

gcc/ChangeLog:

	* config/i386/i386-features.c (remove_partial_avx_dependency):
	Restrict TARGET_USE_VECTOR_FP_CONVERTS and
	TARGET_USE_VECTOR_CONVERTS to conversion instructions only.
---
 gcc/config/i386/i386-features.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

Comments

Jakub Jelinek Sept. 18, 2021, 7:31 a.m. UTC | #1 
On Sat, Sep 18, 2021 at 11:09:32AM +0800, liuhongt wrote:
> Besides conversion instructions, pass_rpad also handles scalar
> sqrt/rsqrt/rcp/round instructions, while r12-3614 should only want to
> handle conversion instructions, so fix it.
> 
>   Bootstrapped and regtest on x86_64-linux-gnu{-m32,} w/ configure
> --enable-checking=yes,rtl,extra, failed tests are fixed.
>   Ok for trunk?
> 
> New tests that PASS (8 tests):
> 
> gcc.target/i386/avx512f-vscalefpd-2.c execution test
> gcc.target/i386/avx512f-vscalefps-2.c execution test
> gcc.target/i386/avx512f-vscalefss-2.c execution test
> gcc.target/i386/avx512fp16-vscalefph-1b.c execution test
> gcc.target/i386/avx512fp16-vscalefsh-1b.c execution test
> gcc.target/i386/avx512fp16vl-vscalefph-1b.c execution test
> gcc.target/i386/avx512vl-vscalefpd-2.c execution test
> gcc.target/i386/avx512vl-vscalefps-2.c execution test
> 
> Old tests that failed, that have disappeared (8 tests): (Eeek!)
> 
> gcc.target/i386/avx512f-vscalefpd-2.c (internal compiler error)
> gcc.target/i386/avx512f-vscalefps-2.c (internal compiler error)
> gcc.target/i386/avx512f-vscalefss-2.c (internal compiler error)
> gcc.target/i386/avx512fp16-vscalefph-1b.c (internal compiler error)
> gcc.target/i386/avx512fp16-vscalefsh-1b.c (internal compiler error)
> gcc.target/i386/avx512fp16vl-vscalefph-1b.c (internal compiler error)
> gcc.target/i386/avx512vl-vscalefpd-2.c (internal compiler error)
> gcc.target/i386/avx512vl-vscalefps-2.c (internal compiler error)
> 
> gcc/ChangeLog:
> 
> 	* config/i386/i386-features.c (remove_partial_avx_dependency):
> 	Restrict TARGET_USE_VECTOR_FP_CONVERTS and
> 	TARGET_USE_VECTOR_CONVERTS to conversion instructions only.

Ok, with a minor nit:

> @@ -2233,6 +2247,7 @@ remove_partial_avx_dependency (void)
>  		continue;
>  	      break;
>  	    default:
> +	      gcc_assert (src_mode == E_VOIDmode);
>  	      break;
>  	    }

Wouldn't it be better to do:
	  E_VOIDmode:
	    gcc_assert (convert_p);
	    break;
	  default:
	    gcc_unreachable ();
?

	Jakub
Hongtao Liu Sept. 18, 2021, 7:56 a.m. UTC | #2 
On Sat, Sep 18, 2021 at 3:31 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Sat, Sep 18, 2021 at 11:09:32AM +0800, liuhongt wrote:
> > Besides conversion instructions, pass_rpad also handles scalar
> > sqrt/rsqrt/rcp/round instructions, while r12-3614 should only want to
> > handle conversion instructions, so fix it.
> >
> >   Bootstrapped and regtest on x86_64-linux-gnu{-m32,} w/ configure
> > --enable-checking=yes,rtl,extra, failed tests are fixed.
> >   Ok for trunk?
> >
> > New tests that PASS (8 tests):
> >
> > gcc.target/i386/avx512f-vscalefpd-2.c execution test
> > gcc.target/i386/avx512f-vscalefps-2.c execution test
> > gcc.target/i386/avx512f-vscalefss-2.c execution test
> > gcc.target/i386/avx512fp16-vscalefph-1b.c execution test
> > gcc.target/i386/avx512fp16-vscalefsh-1b.c execution test
> > gcc.target/i386/avx512fp16vl-vscalefph-1b.c execution test
> > gcc.target/i386/avx512vl-vscalefpd-2.c execution test
> > gcc.target/i386/avx512vl-vscalefps-2.c execution test
> >
> > Old tests that failed, that have disappeared (8 tests): (Eeek!)
> >
> > gcc.target/i386/avx512f-vscalefpd-2.c (internal compiler error)
> > gcc.target/i386/avx512f-vscalefps-2.c (internal compiler error)
> > gcc.target/i386/avx512f-vscalefss-2.c (internal compiler error)
> > gcc.target/i386/avx512fp16-vscalefph-1b.c (internal compiler error)
> > gcc.target/i386/avx512fp16-vscalefsh-1b.c (internal compiler error)
> > gcc.target/i386/avx512fp16vl-vscalefph-1b.c (internal compiler error)
> > gcc.target/i386/avx512vl-vscalefpd-2.c (internal compiler error)
> > gcc.target/i386/avx512vl-vscalefps-2.c (internal compiler error)
> >
> > gcc/ChangeLog:
> >
> >       * config/i386/i386-features.c (remove_partial_avx_dependency):
> >       Restrict TARGET_USE_VECTOR_FP_CONVERTS and
> >       TARGET_USE_VECTOR_CONVERTS to conversion instructions only.
>
> Ok, with a minor nit:
>
> > @@ -2233,6 +2247,7 @@ remove_partial_avx_dependency (void)
> >               continue;
> >             break;
> >           default:
> > +           gcc_assert (src_mode == E_VOIDmode);
> >             break;
> >           }
>
> Wouldn't it be better to do:
>           E_VOIDmode:
>             gcc_assert (convert_p);
!convert_p, Must be typo :)

>             break;
>           default:
>             gcc_unreachable ();
> ?
Sure.
>
>         Jakub
>
Jakub Jelinek Sept. 18, 2021, 7:57 a.m. UTC | #3 
On Sat, Sep 18, 2021 at 03:56:42PM +0800, Hongtao Liu wrote:
> > Wouldn't it be better to do:
> >           E_VOIDmode:
> >             gcc_assert (convert_p);
> !convert_p, Must be typo :)

Yes, sorry.
> 
> >             break;
> >           default:
> >             gcc_unreachable ();
> > ?
> Sure.

	Jakub
diff mbox series

Patch

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index a525a83afd3..001dc9c1053 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -2165,7 +2165,7 @@  make_pass_insert_endbr_and_patchable_area (gcc::context *ctxt)
 }
 
 /* At entry of the nearest common dominator for basic blocks with
-   conversions, generate a single
+   conversions/rcp/sqrt/rsqrt/round, generate a single
 	vxorps %xmmN, %xmmN, %xmmN
    for all
 	vcvtss2sd  op, %xmmN, %xmmX
@@ -2211,13 +2211,27 @@  remove_partial_avx_dependency (void)
 	    continue;
 
 	  /* Convert PARTIAL_XMM_UPDATE_TRUE insns, DF -> SF, SF -> DF,
-	     SI -> SF, SI -> DF, DI -> SF, DI -> DF, to vec_dup and
-	     vec_merge with subreg.  */
+	     SI -> SF, SI -> DF, DI -> SF, DI -> DF, sqrt, rsqrt, rcp,
+	     round, to vec_dup and vec_merge with subreg.  */
 	  rtx src = SET_SRC (set);
 	  rtx dest = SET_DEST (set);
 	  machine_mode dest_mode = GET_MODE (dest);
-	  machine_mode src_mode = GET_MODE (XEXP (src, 0));
+	  bool convert_p = false;
+	  switch (GET_CODE (src))
+	    {
+	    case FLOAT:
+	    case FLOAT_EXTEND:
+	    case FLOAT_TRUNCATE:
+	    case UNSIGNED_FLOAT:
+	      convert_p = true;
+	      break;
+	    default:
+	      break;
+	    }
 
+	  /* Only hanlde conversion here.  */
+	  machine_mode src_mode
+	    = convert_p ? GET_MODE (XEXP (src, 0)) : VOIDmode;
 	  switch (src_mode)
 	    {
 	    case E_SFmode:
@@ -2233,6 +2247,7 @@  remove_partial_avx_dependency (void)
 		continue;
 	      break;
 	    default:
+	      gcc_assert (src_mode == E_VOIDmode);
 	      break;
 	    }