Bug 83221 - [8 Regression] qsort comparator not anti-commutative: -2147483648, -2147483648
Summary: [8 Regression] qsort comparator not anti-commutative: -2147483648, -2147483648
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 8.0
Assignee: Jakub Jelinek
URL:
Keywords: ice-checking, ice-on-valid-code
Depends on:
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2017-11-29 20:00 UTC by Dmitry Babokin
Modified: 2021-11-01 23:07 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-11-30 00:00:00


Attachments
reproducer (200.76 KB, application/x-xz)
2017-11-29 20:00 UTC, Dmitry Babokin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Babokin 2017-11-29 20:00:32 UTC
Created attachment 42748 [details]
reproducer

gcc trunk rev255248, x86_64.

> time g++ -std=c++11 -w -O2 -o gcc_skx_opt_func.o -c func_reduced_gcc_skx_opt.cpp
func_reduced_gcc_skx_opt.cpp: In function ‘void tf_4_foo()’:
func_reduced_gcc_skx_opt.cpp:250:14: error: qsort comparator not anti-commutative: -2147483648, -2147483648
         void tf_4_foo () {
              ^~~~~~~~
during GIMPLE pass: reassoc
func_reduced_gcc_skx_opt.cpp:250:14: internal compiler error: qsort checking failed
0x8f5c3a qsort_chk_error
        ../../gcc/gcc/vec.c:222
0x8f5ca5 qsort_chk(void*, unsigned long, unsigned long, int (*)(void const*, void const*))
        ../../gcc/gcc/vec.c:276
0x7db30b vec<operand_entry*, va_heap, vl_embed>::qsort(int (*)(void const*, void const*))
        ../../gcc/gcc/vec.h:1050
0x7db30b vec<operand_entry*, va_heap, vl_ptr>::qsort(int (*)(void const*, void const*))
        ../../gcc/gcc/vec.h:1812
0x7db30b reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5831
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
0x7dae22 reassociate_bb
        ../../gcc/gcc/tree-ssa-reassoc.c:5983
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Comment 1 Jakub Jelinek 2017-11-30 15:28:13 UTC
The problem is that the testcase contains more than 32768 basic blocks.

One way to fix it is:
2017-11-30  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/83221
	* tree-ssa-reassoc.c (sort_by_operand_rank): Shift bb_rank
	down by 16.
	(init_reassoc): Formatting fix.

--- gcc/tree-ssa-reassoc.c.jj	2017-10-28 09:00:48.000000000 +0200
+++ gcc/tree-ssa-reassoc.c	2017-11-30 16:07:47.220334364 +0100
@@ -543,7 +543,7 @@ sort_by_operand_rank (const void *pa, co
 	    return -1;
 	  /* If neither is, compare bb_rank.  */
 	  if (bb_rank[bbb->index] != bb_rank[bba->index])
-	    return bb_rank[bbb->index] - bb_rank[bba->index];
+	    return (bb_rank[bbb->index] >> 16) - (bb_rank[bba->index] >> 16);
 	}
 
       bool da = reassoc_stmt_dominates_stmt_p (stmta, stmtb);
@@ -6131,7 +6131,7 @@ init_reassoc (void)
 
   /* Set up rank for each BB  */
   for (i = 0; i < n_basic_blocks_for_fn (cfun) - NUM_FIXED_BLOCKS; i++)
-    bb_rank[bbs[i]] = ++rank  << 16;
+    bb_rank[bbs[i]] = ++rank << 16;
 
   free (bbs);
   calculate_dominance_info (CDI_POST_DOMINATORS);

Another possibility is to store bb_rank unshifted and shift only when using bb_rank
except for this sort_by_operand_rank spot.

Perhaps the best fix is to change the types of all the ranks from unsigned int
(used in some spots) and long (in other spots) to uint64_t, on 64-bit hosts it
shouldn't make much difference if we say reorder:
struct operand_entry
{
  unsigned int rank;
  unsigned int id;
  tree op;
  unsigned int count;
  gimple *stmt_to_insert;
};
to
struct operand_entry
{
  unsigned int count;
  unsigned int id;
  tree op;
  uint64_t rank;
  gimple *stmt_to_insert;
};

Guess
static int
compare_repeat_factors (const void *x1, const void *x2)
{
  const repeat_factor *rf1 = (const repeat_factor *) x1;
  const repeat_factor *rf2 = (const repeat_factor *) x2;

  if (rf1->count != rf2->count)
    return rf1->count - rf2->count;

  return rf2->rank - rf1->rank;
}
should be changed in any case to do:
  if (rf1->rank != rf2->rank)
    return rf2->rank > rf1->rank ? 1 : -1;
  return 0;
and likely also the count case.

Richard, thoughts on this?
Comment 2 Jakub Jelinek 2017-12-01 08:14:53 UTC
Author: jakub
Date: Fri Dec  1 08:14:21 2017
New Revision: 255297

URL: https://gcc.gnu.org/viewcvs?rev=255297&root=gcc&view=rev
Log:
	PR tree-optimization/83221
	* tree-ssa-reassoc.c (sort_by_operand_rank): Shift bb_rank
	down by 16.
	(init_reassoc): Formatting fix.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-reassoc.c
Comment 3 Jakub Jelinek 2017-12-01 08:22:53 UTC
Fixed.