Looks like ffmpeg should be added to gcc testsuite. Just another ICE : [/packages/mplayer/libavcodec]> cc -I../libswscale -I../libavcodec -DHAVE_AV_CONFIG_H -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_ISOC9X_SOURCE -I.. -I.. -I../libavutil -Wdisabled-optimization -Wno-pointer-sign -Wdeclaration-after-statement -I. -I.. -I../libavutil -Wall -Wno-switch -Wpointer-arith -Wredundant-decls -O4 -march=native -mtune=native -pipe -ffast-math -fomit-frame-pointer -D_REENTRANT -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -DHAVE_CONFIG_H -I/usr/include/ -I/usr/include/SDL -D_REENTRANT -I/usr/include/freetype2 -I/usr/include -c -o elbg.o elbg.c elbg.c: In function 'try_shift_candidate': elbg.c:245: internal compiler error: Segmentation fault Please submit a full bug report, with preprocessed source if appropriate. See <http://gcc.gnu.org/bugs.html> for instructions. Using built-in specs. Target: i686-pc-linux-gnu Configured with: ../configure --prefix=/usr/lib/gcc-snapshot --disable-libgcj --disable-libssp --disable-nls --disable-werror --disable-checking --enable-clocale=gnu --enable-__cxa_atexit --enable-languages=c,c++,fortran,objc --enable-libstdcxx-allocator=new --enable-shared --enable-ssp --enable-threads=posix --enable-version-specific-runtime-libs --without-included-gettext --without-system-libunwind --with-system-zlib Thread model: posix gcc version 4.3.0 20071005 (experimental) (GCC)
Created attachment 14312 [details] elbg.i produced with -save-temps
Reducing. Inside tree dce we have a statment as a DECL which seems wrong.
Vectorizer produces invalid Gimple SSA code: D.1769_169 = D.1599 /[ex] 4; D.1599 should be renamed.
We do catch this earlier with checking turned on. Reducing all the way now.
Reduced testcase: int f(int dim, int *b) { int newcentroid[3][dim]; int *a = newcentroid[2]; int i, dist=0; for (i=0;i<dim;i++) dist += (a[i] - b[i])*(a[i] - b[i]); return dist; } Compile with -O3 -msse2.
*** Bug 33684 has been marked as a duplicate of this bug. ***
(In reply to comment #3) I get: pr33680.c: In function גfג: pr33680.c:1: error: expected an SSA_NAME object pr33680.c:1: error: in statement D.1618_93 = D.1556 /[ex] 4; pr33680.c:1: internal compiler error: verify_ssa failed The problem is that D.1556 is a VAR_DECL and not an SSA_NAME. This stmt is created while gimplifying data-ref base in vect_create_addr_base_for_vector_ref(). The expr is (int[0:D.1553] *) newcentroid.1_22 + (long unsigned int) dim_4(D) * 8 <pointer_plus_expr 0x2b0e515de8c0 type <pointer_type 0x2b0e515a34e0 type <array_type 0x2b0e515a3270 type <integer_type 0x2b0e514975b0 int> sizes-gimplified type_1 BLK size <var_decl 0x2b0e515586e0 D.1555> unit size <var_decl 0x2b0e51558790 D.1556> ... D.1618 = D.1556 /[ex] 4 is created, taking D.1556 as the unit size in gimplify_compound_lval. And later D.1618 is replaced with an SSA_NAME D.1618_93, since it's a lhs (in gimplify_modify_expr). > Vectorizer produces invalid Gimple SSA code: > > D.1769_169 = D.1599 /[ex] 4; > > D.1599 should be renamed. > Where should it be renamed? In gimplify_smth? Thanks, Ira
If you use force_gimple_operand_bsi, it takes care of that itself. If you e.g. use force_gimple_operand instead, you need to take care of calling mark_symbols_for_renaming yourself.
(In reply to comment #8) > If you use force_gimple_operand_bsi, it takes care of that itself. Thanks! I will try to see if we can use it. The problem is we don't have a bsi, we insert those stmts using bsi_insert_on_edge_immediate on loop_preheader_edge. > If you e.g. use force_gimple_operand instead, you need to take care of > calling mark_symbols_for_renaming yourself. In order to do this, we will have to go through the statement list created by force_gimple_operand, and I am not sure that it's a good idea. Thanks, Ira
Please see http://gcc.gnu.org/ml/gcc-patches/2007-09/msg00155.html (the second patch in it). While the first patch has been committed, if there are other testcases which show that the lack of mark_symbols_for_renaming calls in vectorizer breaks them, IMHO the second patch is needed (and other force_gimple_operand calls in the vectorizer need checking as well).
I understand that those symbols have to be renamed, I am just saying that maybe it should be done in the gimplifier and not in the vectorizer. But since force_gimple_operand_bsi also goes through the statements list, I guess it is reasonable to do the same thing in the vectorizer. Or we can add a new API like force_gimple_operand_and_mark_for_renaming. Anyway, I tried your patch. Now we get a different ICE: internal compiler error: in referenced_var_lookup, at tree-dfa.c:642 D.1556 is marked for renaming but then during update_ssa it cannot find it - htab_find_with_hash (tree-dfa.c:641) returns NULL. #0 referenced_var_lookup (uid=1556) at ../../gcc/gcc/tree-dfa.c:642 #1 0x00000000006f9308 in update_ssa (update_flags=2048) at ../../gcc/gcc/tree-into-ssa.c:3207 #2 0x0000000000aac184 in vect_transform_loop (loop_vinfo=0xe94410) at ../../gcc/gcc/tree-vect-transform.c:7431 #3 0x00000000007fae09 in vectorize_loops () at ../../gcc/gcc/tree-vectorizer.c:2507 #4 0x0000000000631726 in execute_one_pass (pass=0xdfc0c0) at ../../gcc/gcc/passes.c:1116 #5 0x00000000006318ec in execute_pass_list (pass=0xdfc0c0) at ../../gcc/gcc/passes.c:1169 #6 0x00000000006318fe in execute_pass_list (pass=0xdfbee0) at ../../gcc/gcc/passes.c:1170 #7 0x00000000006318fe in execute_pass_list (pass=0xdfb2e0) at ../../gcc/gcc/passes.c:1170 #8 0x00000000007086ce in tree_rest_of_compilation (fndecl=0x2ba807b05800) at ../../gcc/gcc/tree-optimize.c:404 #9 0x000000000088a054 in cgraph_expand_function (node=0x2ba807b05900) at ../../gcc/gcc/cgraphunit.c:1070 #10 0x000000000088bbe7 in cgraph_optimize () at ../../gcc/gcc/cgraphunit.c:1139 #11 0x00000000004144fe in c_write_global_declarations () at ../../gcc/gcc/c-decl.c:8077 #12 0x00000000006ad2e7 in toplev_main (argc=<value optimized out>, argv=<value optimized out>) at ../../gcc/gcc/toplev.c:1052 #13 0x00002ba8077d5154 in __libc_start_main () from /lib64/libc.so.6 #14 0x0000000000403cf9 in _start () Thanks, Ira
The problem is that D.1556 (the var that hasn't been renamed or with the patch can't be looked up) has been deleted. *.phiprop has: long unsigned intD.4 D.1556; ... D.1554_11 = (long unsigned intD.4) dimD.1539_4(D); D.1556_12 = D.1554_11 * 4; ... D.1561_23 = D.1556_12 /[ex] 4; D.1562_24 = &(*newcentroid.1D.1559_22)[0]{lb: 0 sz: D.1561_23 * 4}; then FRE does: Replaced D.1556_12 /[ex] 4 with D.1554_11 in D.1561_23 = D.1556_12 /[ex] 4; long unsigned intD.4 D.1556; ... D.1554_11 = (long unsigned intD.4) dimD.1539_4(D); D.1556_12 = D.1554_11 * 4; ... newcentroid.1D.1559_22 = (intD.0[3][0:D.1553] *) D.1560_21; D.1561_23 = D.1554_11; D.1562_24 = &(*newcentroid.1D.1559_22)[0]{lb: 0 sz: D.1561_23 * 4}; and as D.1556 is unused, it is DCEd in dce2 pass. But D.1556 (and D.1555) is still present in TYPE_SIZE{,_UNIT} of the VLA type, so when vectorizer regimplifies it, it adds back code to compute its size, but that assumes D.1556 has the correct value. Not sure what could be done here to fix this. The easiest solution is not to vectorize when VLA types are involved. In any case it means that debuginfo will be incomplete if its size variable has been DCEd.
Maybe we can fix DCE not to eliminate such vars? Or somehow fix split_constant_offset? The following patch changes the base from (int[0:D.1553] *) newcentroid.1_22 + (long unsigned int) dim_4(D) * 8 to (int[0:D.1553] *) D.1560_21 + (long unsigned int) dim_4(D) * 8 and, hence, there is no need in the size of newcentroid.1_22: Index: tree-data-ref.c =================================================================== --- tree-data-ref.c (revision 128902) +++ tree-data-ref.c (working copy) @@ -579,8 +579,10 @@ split_constant_offset (tree exp, tree *v { split_constant_offset (def_stmt_rhs, &var0, &off0); var0 = fold_convert (type, var0); - *var = var0; - *off = off0; + split_constant_offset (var0, &var2, &off2); + *var = var2; + *off = fold_build2 (PLUS_EXPR, TREE_TYPE (off2), + off0, off2); return; } } Maybe we can check if the base is of the VLA type and then try to further split it as above (and not to vectorize if we fail)? Thanks, Ira
BTW, without this patch http://gcc.gnu.org/ml/gcc-patches/2007-07/msg02122.html there is no ICE and the loop gets vectorized. Ira
(In reply to comment #14) > BTW, without this patch http://gcc.gnu.org/ml/gcc-patches/2007-07/msg02122.html > there is no ICE and the loop gets vectorized. > > Ira > It may be that the test to go through an SSA_NAME in split_constant_offset is still not strict enough. I was having problem knowing what to check for. See thread: http://gcc.gnu.org/ml/gcc-patches/2007-07/msg01294.html --- trunk/gcc/tree-data-ref.c (revision 126953) +++ trunk/gcc/tree-data-ref.c (working copy) @@ -565,6 +565,27 @@ split_constant_offset (tree exp, tree *v return; } + case SSA_NAME: + { + tree def_stmt = SSA_NAME_DEF_STMT (exp); + if (TREE_CODE (def_stmt) == GIMPLE_MODIFY_STMT) + { + tree def_stmt_rhs = GIMPLE_STMT_OPERAND (def_stmt, 1); + + if (!TREE_SIDE_EFFECTS (def_stmt_rhs) + && EXPR_P (def_stmt_rhs) + && !REFERENCE_CLASS_P (def_stmt_rhs)) + { + split_constant_offset (def_stmt_rhs, &var0, &off0); + var0 = fold_convert (type, var0); + *var = var0; + *off = off0; + return;
This patch fixes the ICE and doesn't cause regressions in the vectorizer testsuite: Index: tree-data-ref.c =================================================================== --- tree-data-ref.c (revision 129292) +++ tree-data-ref.c (working copy) @@ -571,11 +571,16 @@ split_constant_offset (tree exp, tree *v if (TREE_CODE (def_stmt) == GIMPLE_MODIFY_STMT) { tree def_stmt_rhs = GIMPLE_STMT_OPERAND (def_stmt, 1); + tree arr = NULL_TREE; + + if (TREE_CODE (def_stmt_rhs) == ADDR_EXPR) + arr = TREE_OPERAND (def_stmt_rhs, 0); if (!TREE_SIDE_EFFECTS (def_stmt_rhs) && EXPR_P (def_stmt_rhs) && !REFERENCE_CLASS_P (def_stmt_rhs) - && !get_call_expr_in (def_stmt_rhs)) + && !get_call_expr_in (def_stmt_rhs) + && (!arr || TREE_THIS_NOTRAP (arr))) { split_constant_offset (def_stmt_rhs, &var0, &off0); var0 = fold_convert (type, var0); This way we avoid arrays with unknown size. Ira
Subject: Re: [4.3 Regression] ICE when compilling elbg.c from ffmpeg (vectorizer) > This patch fixes the ICE and doesn't cause regressions in the vectorizer > testsuite: > > Index: tree-data-ref.c > =================================================================== > --- tree-data-ref.c (revision 129292) > +++ tree-data-ref.c (working copy) > @@ -571,11 +571,16 @@ split_constant_offset (tree exp, tree *v > if (TREE_CODE (def_stmt) == GIMPLE_MODIFY_STMT) > { > tree def_stmt_rhs = GIMPLE_STMT_OPERAND (def_stmt, 1); > + tree arr = NULL_TREE; > + > + if (TREE_CODE (def_stmt_rhs) == ADDR_EXPR) > + arr = TREE_OPERAND (def_stmt_rhs, 0); > > if (!TREE_SIDE_EFFECTS (def_stmt_rhs) > && EXPR_P (def_stmt_rhs) > && !REFERENCE_CLASS_P (def_stmt_rhs) > - && !get_call_expr_in (def_stmt_rhs)) > + && !get_call_expr_in (def_stmt_rhs) > + && (!arr || TREE_THIS_NOTRAP (arr))) you would have to be more careful here, as arr does not have to be array_ref, so applying TREE_THIS_NOTRAP to it may cause ICEs. Anyway, this change does not make sense to me, I need to check what this PR is about, but there is no reason for split_constant_offset to special-case ADDR_EXPR's this way.
*** Bug 33872 has been marked as a duplicate of this bug. ***
*** Bug 33882 has been marked as a duplicate of this bug. ***
What I'd try instead is something like: --- tree-data-ref.c.jj11 2007-10-28 19:34:10.000000000 +0100 +++ tree-data-ref.c 2007-10-31 16:22:21.000000000 +0100 @@ -629,7 +629,7 @@ dr_analyze_innermost (struct data_refere enum machine_mode pmode; int punsignedp, pvolatilep; affine_iv base_iv, offset_iv; - tree init, dinit, step; + tree init, dinit, step, type; if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "analyze_innermost: "); @@ -666,9 +666,21 @@ dr_analyze_innermost (struct data_refere init = ssize_int (pbitpos / BITS_PER_UNIT); split_constant_offset (base_iv.base, &base_iv.base, &dinit); - init = size_binop (PLUS_EXPR, init, dinit); + init = size_binop (PLUS_EXPR, init, dinit); split_constant_offset (offset_iv.base, &offset_iv.base, &dinit); - init = size_binop (PLUS_EXPR, init, dinit); + init = size_binop (PLUS_EXPR, init, dinit); + + /* See if base address involves a variable length type somewhere. + Bases with such types shouldn't be gimplified again. */ + type = TREE_TYPE (base_iv.base); + while (POINTER_TYPE_P (type)) + type = TREE_TYPE (type); + if (int_size_in_bytes (type) < 0) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, "failed: base involves a variable length type.\n"); + return; + } step = size_binop (PLUS_EXPR, fold_convert (ssizetype, base_iv.step), or similar check in the vectorizer instead. This exact patch breaks gfortran.dg/vect/vect-3.f90 and gfortran.dg/vect/pr19049.f90 (they are no longer vectorized as expected), would need to see whether it tries to unsafely gimplify the VLA type sizes in the vectorizer or not.
Subject: Bug 33680 Author: jakub Date: Sat Nov 10 07:40:37 2007 New Revision: 130067 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=130067 Log: PR tree-optimization/33680 * tree-data-ref.c (split_constant_offset) <case ADDR_EXPR>: Punt if the added cast involves variable length types. * gcc.c-torture/compile/20071108-1.c: New test. Added: trunk/gcc/testsuite/gcc.c-torture/compile/20071108-1.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-data-ref.c
Fixed.