Bug 89223 - [7 Regression] internal compiler error: in int_cst_value, at tree.c:11226
Summary: [7 Regression] internal compiler error: in int_cst_value, at tree.c:11226
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 9.0
: P2 normal
Target Milestone: 7.5
Assignee: Richard Biener
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2019-02-06 12:14 UTC by Sameeran Joshi
Modified: 2019-03-26 13:20 UTC (History)
2 users (show)

See Also:
Host: x86_64-linux-gnu
Target: x86_64-linux-gnu
Build: x86_64-linux-gnu
Known to work: 7.4.1, 8.2.1, 9.0
Known to fail: 7.4.0, 8.2.0
Last reconfirmed: 2019-02-06 00:00:00


Attachments
Preprocessed code of file named "crash1.c" (50.51 KB, text/plain)
2019-02-06 12:14 UTC, Sameeran Joshi
Details
patch I am testing (1.03 KB, patch)
2019-02-07 09:10 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sameeran Joshi 2019-02-06 12:14:46 UTC
Created attachment 45620 [details]
Preprocessed code of file named "crash1.c"

------------------------------BACKTRACK---------------------------------

during GIMPLE pass: pcom
crash1.c: In function ‘func_13.constprop’:
crash1.c:701:24: internal compiler error: in int_cst_value, at tree.c:11730
  701 | static const int32_t * func_13(int32_t * p_14)
      |                        ^~~~~~~
0x1fa2edc int_cst_value(tree_node const*)
        ../../gcc/tree.c:11730
0x31bd717 initialize_matrix_A
        ../../gcc/tree-data-ref.c:3194
0x31bf66d analyze_subscript_affine_affine
        ../../gcc/tree-data-ref.c:3614
0x31c15ff analyze_siv_subscript
        ../../gcc/tree-data-ref.c:3925
0x31c2b16 analyze_overlapping_iterations
        ../../gcc/tree-data-ref.c:4171
0x31c4e49 subscript_dependence_tester_1
        ../../gcc/tree-data-ref.c:4713
0x31c5074 subscript_dependence_tester
        ../../gcc/tree-data-ref.c:4760
0x31c54c9 compute_affine_dependence(data_dependence_relation*, loop*)
        ../../gcc/tree-data-ref.c:4819
0x31c5a3f compute_all_dependences(vec<data_reference*, va_heap,
vl_ptr>, vec<data_dependence_relation*, va_heap, vl_ptr>*, vec<loop*,
va_heap, vl_ptr>, bool)
        ../../gcc/tree-data-ref.c:4886
0x31c7daf compute_data_dependences_for_loop(loop*, bool, vec<loop*,
va_heap, vl_ptr>*, vec<data_reference*, va_heap, vl_ptr>*,
vec<data_dependence_relation*, va_heap, vl_ptr>*)
        ../../gcc/tree-data-ref.c:5360
0x1a465ac tree_predictive_commoning_loop
        ../../gcc/tree-predcom.c:3191
0x1a47190 tree_predictive_commoning()
        ../../gcc/tree-predcom.c:3313
0x1a472eb run_tree_predictive_commoning
        ../../gcc/tree-predcom.c:3338
0x1a473b7 execute
        ../../gcc/tree-predcom.c:3367


---------------------------COMMMAND LINE--------------------------------
~/again_build/bin/current-gcc -O3 -fgnu-tm crash1.c -I../../runtime/ -w

----------------------------GCC VERSION----------------------------------
current-gcc (GCC) 9.0.0 20190108 (experimental)


--------------------------PREVIOUS REPORTING-------------------------------

https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&bug_status=RESOLVED&bug_status=VERIFIED&bug_status=CLOSED&cf_known_to_fail_type=allwords&cf_known_to_work_type=allwords&list_id=228813&query_format=advanced&short_desc=internal%20compiler%20error%3A%20in%20int_cst_value&short_desc_type=allwordssubstr

---------------------------REDUCED TEST CASE ----------------------------

a[5];
unsigned __int128 b;
c() {
  b = 4;
  for (;; b--)
    a[b] = ({ a[b + b]; });
}

Previous reporting were on older version and reduced test case were different. So I filed a new bug because it fails on the trunk.
Comment 1 David Malcolm 2019-02-06 15:22:09 UTC
Confirmed via godbolt; this crashes GCC 5 onwards, including trunk.

It's failing this assertion here:

11731	  /* Make sure the sign-extended value will fit in a HOST_WIDE_INT.  */
11732	  gcc_assert (cst_and_fits_in_hwi (x));

where:

(gdb) call debug_tree (x)
 <integer_cst 0x7ffff1a15550 type <integer_type 0x7ffff18c6a80 __int128 unsigned> constant 0xfffffffffffffffffffffffffffffffe>
Comment 2 Jakub Jelinek 2019-02-06 15:33:50 UTC
Updated testcase that doesn't have UB at runtime if it was ever called:
int a[9];
unsigned __int128 b;

void
foo (void)
{
  for (b = 4; b != 0; b--)
    a[b] = a[b + b];
}
Comment 3 Jakub Jelinek 2019-02-06 15:50:51 UTC
Started with r210113.
int_cst_value changed there:
   /* Make sure the sign-extended value will fit in a HOST_WIDE_INT.  */
-  gcc_assert (TREE_INT_CST_HIGH (x) == 0
-	      || TREE_INT_CST_HIGH (x) == -1);
+  gcc_assert (cst_and_fits_in_hwi (x));
which is not equivalent, as previously it wasn't testing whether it has signed or unsigned type, now it is.

With a small modification:
int a[9];
unsigned __int128 b;

void
foo (void)
{
  for (b = (((unsigned __int128) 4) << 64) + 4; b != 0; b -= ((((unsigned __int128) 1) << 64) + 1))
    a[b] = a[b + b];
}
(where it relies on sizetype/pointers being at most 64-bit), it started to ICE with r159879 when __int128 support has been introduced.
Comment 4 Jakub Jelinek 2019-02-06 16:08:32 UTC
I guess either we should lower ARRAY_REF indexes with precisions higher than pointer precision to that smaller precision early (during gimplification e.g.?), though not really sure what effect would that have on weirdo targets with 20/24-bit etc. pointers and sizetype say even smaller than pointers, or we should just punt somewhere in the data dependence handling for anything that involves integers with > HOST_BITS_PER_WIDE_INT precisions.
Comment 5 Jakub Jelinek 2019-02-06 16:23:16 UTC
The gimplifier change would be something like:
--- gimplify.c.jj	2019-01-28 23:30:53.199762928 +0100
+++ gimplify.c	2019-02-06 17:15:35.368976785 +0100
@@ -2977,6 +2977,10 @@ gimplify_compound_lval (tree *expr_p, gi
 
       if (TREE_CODE (t) == ARRAY_REF || TREE_CODE (t) == ARRAY_RANGE_REF)
 	{
+	  if (!error_operand_p (TREE_OPERAND (t, 1))
+	      && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (t, 1)))
+		  > TYPE_PRECISION (sizetype)))
+	    TREE_OPERAND (t, 1) = fold_convert (sizetype, TREE_OPERAND (t, 1));
 	  /* Gimplify the dimension.  */
 	  if (!is_gimple_min_invariant (TREE_OPERAND (t, 1)))
 	    {
and besides the fears about weirdo targets I think it is the right thing even on 32-bit targets when they use long long indexes into arrays.  After all, if we gimplify it into pointer arithmetics, we'd cast to sizetype anyway.
Richard, thoughts on this?
Or if we fear too much, we could do that only if pointers have the same precision as sizetype or something similar, I think weirdo targets will not have support for > 64-bit integral types anyway.
Comment 6 Richard Biener 2019-02-07 08:47:54 UTC
The error is clearly in the dataref code, first that it ends up asserting instead of failing analysis and second that it, dependent on context(!) keeps to-be
interpreted as "signed" values sizetype constants in (unsigned) sizetype.

Papering over this in the gimplifier shouldn't be done.  Fixing it "properly"
is on my longer TODO list but with low priority give it triggers only in these
kind of weird circumstances (and the issue is so old).
Comment 7 Richard Biener 2019-02-07 08:51:40 UTC
The chrec in question is {8, +, 0xfffffffffffffffffffffffffffffffe}_1 which
wraps.  The dependence code basically assumes infinite precision integers
(which we don't have) and thus cannot really handle the case of wrapping IVs.
Comment 8 Richard Biener 2019-02-07 09:01:37 UTC
That is, I would say a better fix would be to analyze the chrec
as {4, +, -2}, thus in another type in the first place.

But then the dataref code has to deal with failures in the analysis
(there are also overflow conditions that are more or less correctly
handled).  You can see initialize_matrix_A results when not recursing
are immediately fed into int_cst_value - where there is also a possible
failure mode outlined:

          if (dump_file && (dump_flags & TDF_DETAILS))
            fprintf (dump_file, "affine-affine test failed: too many variables.\n");
          *overlaps_a = conflict_fn_not_known ();
          *overlaps_b = conflict_fn_not_known ();
          *last_conflicts = chrec_dont_know;

so all is necessary is to check cst_and_fits_in_hwi and propagate the error
upward from initialize_matrix_A.
Comment 9 Richard Biener 2019-02-07 09:10:32 UTC
Created attachment 45624 [details]
patch I am testing
Comment 10 Jakub Jelinek 2019-02-07 16:59:57 UTC
The #c9 patch looks good, but I don't view #c5 as papering over issues, but rather as an optimization and desirable change.
The expansion of ARRAY_REFs is done through calling get_inner_reference and expanding the base and offset.
And, get_inner_reference does with the index:
            offset = size_binop (PLUS_EXPR, offset,
                                 size_binop (MULT_EXPR,
                                             fold_convert (sizetype, index),
                                             unit_size));
so, all upper bits beyond TYPE_PRECISION (sizetype) are thrown away.
While it is probably desirable for optimization purposes to keep ARRAY_REF indexes with smaller or equal precision than sizetype in whatever type they were originally, I don't see any advantages of pretending we care about the extra bits we ignore in the end, by folding it during gimplification we might generate better code by computing stuff only in narrower types etc.
Given above, I'm no longer worried about what it would cause for targets with weird pointer sizes.
BTW, e.g. get_addr_base_and_unit_offset_1 doesn't seem to be prepared to handle ARRAY_REF indexes larger than sizetype:
                poly_offset_int woffset
                  = wi::sext (wi::to_poly_offset (index)
                              - wi::to_poly_offset (low_bound),
                              TYPE_PRECISION (TREE_TYPE (index)));
While offset_int is actually 128-bit for efficiency, if we use the full sometimes signed, sometimes unsigned __int128 indexes there, we don't have the always signed bit anyway.  Guess the above could be easily changed to
MIN (TYPE_PRECISION (TREE_TYPE (index)), TYPE_PRECISION (sizetype)) or similar,
but with the gimplifier change it wouldn't be needed (we could later on add verifier that ARRAY*REF indexes aren't wider than sizetype's precision).
Comment 11 Richard Biener 2019-02-08 08:18:40 UTC
Author: rguenth
Date: Fri Feb  8 08:18:09 2019
New Revision: 268666

URL: https://gcc.gnu.org/viewcvs?rev=268666&root=gcc&view=rev
Log:
2019-02-08  Richard Biener  <rguenther@suse.de>

	PR middle-end/89223
	* tree-data-ref.c (initialize_matrix_A): Fail if constant
	doesn't fit in HWI.
	(analyze_subscript_affine_affine): Handle failure from
	initialize_matrix_A.

	* gcc.dg/torture/pr89223.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr89223.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-data-ref.c
Comment 12 Richard Biener 2019-02-13 10:03:19 UTC
Author: rguenth
Date: Wed Feb 13 10:02:47 2019
New Revision: 268838

URL: https://gcc.gnu.org/viewcvs?rev=268838&root=gcc&view=rev
Log:
2019-02-13  Richard Biener  <rguenther@suse.de>

	Backport from mainline
	2019-02-12  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/89253
	* tree-ssa-loop-split.c (tree_ssa_split_loops): Check we can
	duplicate the loop.

	* gfortran.dg/pr89253.f: New testcase.

	2019-02-08  Richard Biener  <rguenther@suse.de>

	PR middle-end/89223
	* tree-data-ref.c (initialize_matrix_A): Fail if constant
	doesn't fit in HWI.
	(analyze_subscript_affine_affine): Handle failure from
	initialize_matrix_A.

	* gcc.dg/torture/pr89223.c: New testcase.

	2019-01-28  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/88739
	* tree-ssa-sccvn.c (vn_reference_lookup_3): Avoid generating
	BIT_FIELD_REFs of non-mode-precision integral operands.

	* gcc.c-torture/execute/pr88739.c: New test.

Added:
    branches/gcc-8-branch/gcc/testsuite/gcc.c-torture/execute/pr88739.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/torture/pr89223.c
    branches/gcc-8-branch/gcc/testsuite/gfortran.dg/pr89253.f
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
    branches/gcc-8-branch/gcc/tree-data-ref.c
    branches/gcc-8-branch/gcc/tree-ssa-loop-split.c
    branches/gcc-8-branch/gcc/tree-ssa-sccvn.c
Comment 13 Richard Biener 2019-03-26 13:18:56 UTC
Author: rguenth
Date: Tue Mar 26 13:18:23 2019
New Revision: 269942

URL: https://gcc.gnu.org/viewcvs?rev=269942&root=gcc&view=rev
Log:
2019-02-26  Richard Biener  <rguenther@suse.de>

	Backport from mainline
	2019-02-12  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/89253
	* tree-ssa-loop-split.c (tree_ssa_split_loops): Check we can
	duplicate the loop.

	* gfortran.dg/pr89253.f: New testcase.

	2019-02-08  Richard Biener  <rguenther@suse.de>

	PR middle-end/89223
	* tree-data-ref.c (initialize_matrix_A): Fail if constant
	doesn't fit in HWI.
	(analyze_subscript_affine_affine): Handle failure from
	initialize_matrix_A.

	* gcc.dg/torture/pr89223.c: New testcase.

	2019-01-28  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/88739
	* tree-ssa-sccvn.c (vn_reference_lookup_3): Avoid generating
	BIT_FIELD_REFs of non-mode-precision integral operands.

	* gcc.c-torture/execute/pr88739.c: New test.

Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.c-torture/execute/pr88739.c
    branches/gcc-7-branch/gcc/testsuite/gcc.dg/torture/pr89223.c
    branches/gcc-7-branch/gcc/testsuite/gfortran.dg/pr89253.f
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
    branches/gcc-7-branch/gcc/tree-data-ref.c
    branches/gcc-7-branch/gcc/tree-ssa-loop-split.c
    branches/gcc-7-branch/gcc/tree-ssa-sccvn.c
Comment 14 Richard Biener 2019-03-26 13:20:56 UTC
Fixed.