diff mbox

Drop CONSTRUCTOR comparsion from ipa-icf-gimple

Message ID 20151016031234.GC45365@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Oct. 16, 2015, 3:12 a.m. UTC
Hi,
as Richard noticed in my port of the code to operand_equal_p, the checking of
CONSTURCTOR in ipa-icf-gimple is incomplete missing the index checks.
It is also unnecesary since non-empty ctors does not happen as gimple
operands.  This patch thus removes the unnecesary code.

Bootstrapped/regtested x86_64-linux, comitted.

Honza

	* ipa-icf-gimple.c (func_checker::compare_operand): Compare only
	empty constructors.

Comments

Richard Biener Oct. 16, 2015, 8:46 a.m. UTC | #1
On Fri, Oct 16, 2015 at 5:12 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> as Richard noticed in my port of the code to operand_equal_p, the checking of
> CONSTURCTOR in ipa-icf-gimple is incomplete missing the index checks.
> It is also unnecesary since non-empty ctors does not happen as gimple
> operands.  This patch thus removes the unnecesary code.

Err - they do happen, for vector constructors.  Just empty constructors
are not allowed for vector constructors - vector constructors are required
to have elements in proper order and none left out.

Sorry for misleading you.

> Bootstrapped/regtested x86_64-linux, comitted.

this will definitely ICE ...

Richard.

> Honza
>
>         * ipa-icf-gimple.c (func_checker::compare_operand): Compare only
>         empty constructors.
> Index: ipa-icf-gimple.c
> ===================================================================
> --- ipa-icf-gimple.c    (revision 228851)
> +++ ipa-icf-gimple.c    (working copy)
> @@ -415,20 +415,9 @@ func_checker::compare_operand (tree t1,
>    switch (TREE_CODE (t1))
>      {
>      case CONSTRUCTOR:
> -      {
> -       unsigned length1 = vec_safe_length (CONSTRUCTOR_ELTS (t1));
> -       unsigned length2 = vec_safe_length (CONSTRUCTOR_ELTS (t2));
> -
> -       if (length1 != length2)
> -         return return_false ();
> -
> -       for (unsigned i = 0; i < length1; i++)
> -         if (!compare_operand (CONSTRUCTOR_ELT (t1, i)->value,
> -                               CONSTRUCTOR_ELT (t2, i)->value))
> -           return return_false();
> -
> -       return true;
> -      }
> +      gcc_assert (!vec_safe_length (CONSTRUCTOR_ELTS (t1))
> +                 && !vec_safe_length (CONSTRUCTOR_ELTS (t2)));
> +      return true;
>      case ARRAY_REF:
>      case ARRAY_RANGE_REF:
>        /* First argument is the array, second is the index.  */
H.J. Lu Oct. 16, 2015, 10:50 a.m. UTC | #2
On Fri, Oct 16, 2015 at 1:46 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Fri, Oct 16, 2015 at 5:12 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi,
>> as Richard noticed in my port of the code to operand_equal_p, the checking of
>> CONSTURCTOR in ipa-icf-gimple is incomplete missing the index checks.
>> It is also unnecesary since non-empty ctors does not happen as gimple
>> operands.  This patch thus removes the unnecesary code.
>
> Err - they do happen, for vector constructors.  Just empty constructors
> are not allowed for vector constructors - vector constructors are required
> to have elements in proper order and none left out.
>
> Sorry for misleading you.
>
>> Bootstrapped/regtested x86_64-linux, comitted.
>
> this will definitely ICE ...
>

And it did on x86:

https://gcc.gnu.org/ml/gcc-regression/2015-10/msg00166.html

FAIL: gcc.dg/pr63914.c (internal compiler error)
FAIL: gcc.dg/pr63914.c (test for excess errors)
FAIL: gcc.target/i386/avx-1.c (internal compiler error)
FAIL: gcc.target/i386/avx-1.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v16sf-1.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v16sf-1.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v16sf-2.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v16sf-2.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v16sf-3.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v16sf-3.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v16si-1.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v16si-1.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v16si-2.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v16si-2.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v16si-3.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v16si-3.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v8df-1.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v8df-1.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v8df-2.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v8df-2.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v8df-3.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v8df-3.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v8di-1.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v8di-1.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v8di-2.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v8di-2.c (test for excess errors)
FAIL: gcc.target/i386/avx512f-set-v8di-3.c (internal compiler error)
FAIL: gcc.target/i386/avx512f-set-v8di-3.c (test for excess errors)
FAIL: gcc.target/i386/sse-13.c (internal compiler error)
FAIL: gcc.target/i386/sse-13.c (test for excess errors)
FAIL: gcc.target/i386/sse-18.c (internal compiler error)
FAIL: gcc.target/i386/sse-18.c (test for excess errors)
FAIL: gcc.target/i386/sse-19.c (internal compiler error)
FAIL: gcc.target/i386/sse-19.c (test for excess errors)
FAIL: gcc.target/i386/sse-23.c (internal compiler error)
FAIL: gcc.target/i386/sse-23.c (test for excess errors)
FAIL: gcc.target/i386/sse-24.c (internal compiler error)
FAIL: gcc.target/i386/sse-24.c (test for excess errors)
FAIL: gcc.target/i386/sse-25.c (internal compiler error)
FAIL: gcc.target/i386/sse-25.c (test for excess errors)
FAIL: gcc.target/i386/vecinit-1.c (internal compiler error)
FAIL: gcc.target/i386/vecinit-1.c (test for excess errors)
FAIL: gcc.target/i386/vecinit-2.c (internal compiler error)
FAIL: gcc.target/i386/vecinit-2.c (test for excess errors)
FAIL: gcc.target/i386/vecinit-5.c (internal compiler error)
FAIL: gcc.target/i386/vecinit-5.c (test for excess errors)
FAIL: gcc.target/i386/vecinit-6.c (internal compiler error)
FAIL: gcc.target/i386/vecinit-6.c (test for excess errors)
diff mbox

Patch

Index: ipa-icf-gimple.c
===================================================================
--- ipa-icf-gimple.c	(revision 228851)
+++ ipa-icf-gimple.c	(working copy)
@@ -415,20 +415,9 @@  func_checker::compare_operand (tree t1,
   switch (TREE_CODE (t1))
     {
     case CONSTRUCTOR:
-      {
-	unsigned length1 = vec_safe_length (CONSTRUCTOR_ELTS (t1));
-	unsigned length2 = vec_safe_length (CONSTRUCTOR_ELTS (t2));
-
-	if (length1 != length2)
-	  return return_false ();
-
-	for (unsigned i = 0; i < length1; i++)
-	  if (!compare_operand (CONSTRUCTOR_ELT (t1, i)->value,
-				CONSTRUCTOR_ELT (t2, i)->value))
-	    return return_false();
-
-	return true;
-      }
+      gcc_assert (!vec_safe_length (CONSTRUCTOR_ELTS (t1))
+		  && !vec_safe_length (CONSTRUCTOR_ELTS (t2)));
+      return true;
     case ARRAY_REF:
     case ARRAY_RANGE_REF:
       /* First argument is the array, second is the index.  */