Bug 91204 - [10 Regression] ICE in expand_expr_real_2, at expr.c:9215 with -O3
Summary: [10 Regression] ICE in expand_expr_real_2, at expr.c:9215 with -O3
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 10.0
Assignee: Jakub Jelinek
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2019-07-18 19:19 UTC by Vsevolod Livinskii
Modified: 2021-11-01 23:07 UTC (History)
3 users (show)

See Also:
Host:
Target: x86_64-*-* i?86-*-*
Build:
Known to work:
Known to fail: 10.0
Last reconfirmed: 2019-07-18 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vsevolod Livinskii 2019-07-18 19:19:04 UTC
GCC fails with ICE in expand_expr_real_2, at expr.c:9215.

Reproducer:

int a, b;
extern int c[];
void d() {
  for (int e = 6; e <= a; e++)
    c[e] &= b ^ c[e] ^ c[e - 2];
}

Error:
>$ gcc -c -O3 small.c
during RTL pass: expand
small.c: In function ‘d’:
small.c:5:10: internal compiler error: in expand_expr_real_2, at expr.c:9215
    5 |     c[e] &= b ^ c[e] ^ c[e - 2];
      |          ^~
0x8c9408 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier)
        /gcc/expr.c:9215
0x8d0532 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        /gcc/expr.c:9948
0x8dac5e expand_expr
        /gcc/expr.h:281
0x8dac5e expand_operands(tree_node*, tree_node*, rtx_def*, rtx_def**, rtx_def**, expand_modifier)
        /gcc/expr.c:7878
0x8c829d expand_expr_real_2(separate_ops*, rtx_def*, machine_mode, expand_modifier)
        /gcc/expr.c:9738
0x796a60 expand_gimple_stmt_1
        /gcc/cfgexpand.c:3814
0x796a60 expand_gimple_stmt
        /gcc/cfgexpand.c:3875
0x79da4f expand_gimple_basic_block
        /gcc/cfgexpand.c:5915
0x7a027e execute
        /gcc/cfgexpand.c:6538

GCC version is 10.0.0 (rev. 273556)
Comment 1 Jakub Jelinek 2019-07-18 22:11:17 UTC
Started with r272711.
Comment 2 Jakub Jelinek 2019-07-18 22:26:25 UTC
The vectorizer vectorizes the loop using 8-byte vectors, but then match.pd during forwprop4 optimizes:
-  vect__6.17_46 = _52 ^ vect__1.12_39;
-  vect__7.18_47 = vect__6.17_46 & vect__1.12_39;
+  _27 = ~_52;
+  vect__7.18_47 = _27 & vect__1.12_39;
using the:
/* Fold (X & Y) ^ Y and (X ^ Y) & Y as ~X & Y.  */
(for opo (bit_and bit_xor)
     opi (bit_xor bit_and)
 (simplify
  (opo:c (opi:cs @0 @1) @1)
  (bit_and (bit_not @0) @1)))
rule, but does that without checking whether the one_cmpl optab is available for the vector mode, and that is done after vector lowering.
The options I see are, either add some optab checking to match.pd patterns when working on vector types, at least when post vector lowering, or simply ensure that all backends have one_cmpl expanders for modes for which they have xor instructions, or teach the expander to fallback to using xor if they need one_cmpl optab which isn't available and xor is.  The last one seems like the best option to me.
Comment 3 Jakub Jelinek 2019-07-18 22:43:00 UTC
--- gcc/optabs.c.jj	2019-07-15 10:53:10.743205405 +0200
+++ gcc/optabs.c	2019-07-19 00:38:20.271852242 +0200
@@ -2972,6 +2972,17 @@ expand_unop (machine_mode mode, optab un
       return target;
     }
 
+  /* Emit ~op0 as op0 ^ -1.  */
+  if (unoptab == one_cmpl_optab
+      && (SCALAR_INT_MODE_P (mode) || GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
+      && optab_handler (xor_optab, mode) != CODE_FOR_nothing)
+    {
+      temp = expand_binop (mode, xor_optab, op0, CONSTM1_RTX (mode),
+			   target, unsignedp, OPTAB_DIRECT);
+      if (temp)
+	return temp;
+    }
+
   if (optab_to_code (unoptab) == NEG)
     {
       /* Try negating floating point values by flipping the sign bit.  */

seems to work for me.  Or of course something similar can be done in config/i386/mmx.md, basically copy the sse.md one_cmpl<mode>2 pattern to mmx.md with TARGET_MMX_WITH_SSE and MMXMODEI iterator.
Comment 4 Richard Biener 2019-07-19 08:13:53 UTC
(In reply to Jakub Jelinek from comment #3)
> --- gcc/optabs.c.jj	2019-07-15 10:53:10.743205405 +0200
> +++ gcc/optabs.c	2019-07-19 00:38:20.271852242 +0200
> @@ -2972,6 +2972,17 @@ expand_unop (machine_mode mode, optab un
>        return target;
>      }
>  
> +  /* Emit ~op0 as op0 ^ -1.  */
> +  if (unoptab == one_cmpl_optab
> +      && (SCALAR_INT_MODE_P (mode) || GET_MODE_CLASS (mode) ==
> MODE_VECTOR_INT)
> +      && optab_handler (xor_optab, mode) != CODE_FOR_nothing)
> +    {
> +      temp = expand_binop (mode, xor_optab, op0, CONSTM1_RTX (mode),
> +			   target, unsignedp, OPTAB_DIRECT);
> +      if (temp)
> +	return temp;
> +    }
> +
>    if (optab_to_code (unoptab) == NEG)
>      {
>        /* Try negating floating point values by flipping the sign bit.  */
> 
> seems to work for me.  Or of course something similar can be done in
> config/i386/mmx.md, basically copy the sse.md one_cmpl<mode>2 pattern to
> mmx.md with TARGET_MMX_WITH_SSE and MMXMODEI iterator.

Doing it in the middle-end is better IMHO.  But yeah, we're somewhat sloppy
with vector optabs checks in match.pd where it seems "obvious" targets need
to have support...
Comment 5 Richard Sandiford 2019-07-19 08:18:42 UTC
> Doing it in the middle-end is better IMHO.  But yeah, we're somewhat sloppy
> with vector optabs checks in match.pd where it seems "obvious" targets need
> to have support...

+1 FWIW.  The patch in comment 3 looks good.
Comment 6 Uroš Bizjak 2019-07-19 09:28:06 UTC
(In reply to Jakub Jelinek from comment #3)
> seems to work for me.  Or of course something similar can be done in
> config/i386/mmx.md, basically copy the sse.md one_cmpl<mode>2 pattern to
> mmx.md with TARGET_MMX_WITH_SSE and MMXMODEI iterator.

Like this?

--cut here--
diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md
index 4c71e66e6607..c78b33b510a6 100644
--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -1158,6 +1158,14 @@
 ;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
+(define_expand "one_cmpl<mode>2"
+  [(set (match_operand:MMXMODEI 0 "register_operand")
+       (xor:MMXMODEI
+         (match_operand:MMXMODEI 1 "register_operand")
+         (match_dup 2)))]
+  "TARGET_MMX_WITH_SSE"
+  "operands[2] = force_reg (<MODE>mode, CONSTM1_RTX (<MODE>mode));")
+
 (define_insn "mmx_andnot<mode>3"
   [(set (match_operand:MMXMODEI 0 "register_operand" "=y,x,Yv")
        (and:MMXMODEI
--cut here--
Comment 7 Jakub Jelinek 2019-07-19 09:31:53 UTC
Yes.  But #c3 does the same thing in the middle-end.
Though, if there is a backend pattern, it will be used also by the vectorizer or taken into account by generic vector lowering.
So maybe we want both?
Comment 8 Uroš Bizjak 2019-07-19 09:33:43 UTC
(In reply to Jakub Jelinek from comment #7)
> Yes.  But #c3 does the same thing in the middle-end.
> Though, if there is a backend pattern, it will be used also by the
> vectorizer or taken into account by generic vector lowering.
> So maybe we want both?

Yes, this was also my intention. A named pattern can be used elsewhere, too.
Comment 9 uros 2019-07-19 14:37:21 UTC
Author: uros
Date: Fri Jul 19 14:36:49 2019
New Revision: 273604

URL: https://gcc.gnu.org/viewcvs?rev=273604&root=gcc&view=rev
Log:
	PR target/91204
	* config/i386/mmx.md (one_cmpl<mode>2): New expander.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/mmx.md
Comment 10 Jakub Jelinek 2019-07-20 17:13:33 UTC
Author: jakub
Date: Sat Jul 20 17:13:00 2019
New Revision: 273629

URL: https://gcc.gnu.org/viewcvs?rev=273629&root=gcc&view=rev
Log:
	PR target/91204
	* optabs.c (expand_unop): As fallback, expand ~op0 as op0 ^ -1.

	* gcc.c-torture/compile/pr91204.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr91204.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/optabs.c
    trunk/gcc/testsuite/ChangeLog
Comment 11 Jakub Jelinek 2019-07-20 18:40:30 UTC
Fixed.