On 07/08/2014 15:18, Roman Gareev wrote:
To commit 2), I would like you to run a wider set of tests (e.g., the LLVM
test suite). If this passes successful, we should give a headsup on the GCC
mailing list and ask other people to try the new isl support.
If now bugs have found, we switch.
I've tested the modified version of Graphite using the LLVM test suite
and found out regression only in two test cases:
/llvm-test-suite/MultiSource/Benchmarks/ASC_Sequoia/CrystalMk/Crystal_Cholesky.c,
/llvm-test suite/MultiSource/Benchmarks/Bullet
Very good news.
Compilation of
/llvm-test-suite/MultiSource/Benchmarks/ASC_Sequoia/CrystalMk/Crystal_Cholesky.c
produces the following error message:
/home/roman/llvm-test-suite/MultiSource/Benchmarks/ASC_Sequoia/CrystalMk/Crystal_Cholesky.c
/home/roman/llvm-test-suite/MultiSource/Benchmarks/Bullet
Crystal_Cholesky.c: In function ‘Crystal_Cholesky’:
Crystal_Cholesky.c:16:6: internal compiler error: in operator[], at vec.h:736
void Crystal_Cholesky(int nSlip,
^
0xf1d080 vec<tree_node*, va_heap, vl_embed>::operator[](unsigned int)
/home/roman/sec_trunk/gcc/gcc/vec.h:736
0xf1d080 vec<tree_node*, va_heap, vl_ptr>::operator[](unsigned int)
/home/roman/sec_trunk/gcc/gcc/vec.h:1202
0xf1d080 build_iv_mapping
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:608
0xf1d080 translate_isl_ast_node_user
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:644
0xf1d080 translate_isl_ast
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:734
0xf1c7d0 translate_isl_ast_for_loop
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:441
0xf1c7d0 translate_isl_ast_node_for
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:580
0xf1c7d0 translate_isl_ast
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:727
0xf1c3e0 translate_isl_ast_node_block
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:669
0xf1c3e0 translate_isl_ast
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:738
0xf1c3e0 translate_isl_ast_node_block
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:669
0xf1c3e0 translate_isl_ast
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:738
0xf1c3e0 translate_isl_ast_node_block
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:669
0xf1c3e0 translate_isl_ast
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:738
0xf1c3e0 translate_isl_ast_node_block
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:669
0xf1c3e0 translate_isl_ast
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:738
0xf1c3e0 translate_isl_ast_node_block
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:669
0xf1c3e0 translate_isl_ast
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:738
0xf1c7d0 translate_isl_ast_for_loop
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:441
0xf1c7d0 translate_isl_ast_node_for
/home/roman/sec_trunk/gcc/gcc/graphite-isl-ast-to-gimple.c:580
It is caused by the wrong index of the iv_map (its value is 12), which
is greater than its size (its value is equal to loop->num = 8 plus
one). I think that we should use the value of number_of_loops for the
size of the iv_map (the attached patch implements this), because it is
proven to be robust. What do you think about this?
I think this is fine. On the other side, I think the test case itself
is unnecessarily large. I would assume the majority of the code could
be deleted while still triggering the bug. Could you give it a shot.
The second error is produced by the executable, which is successfully
compiled by /llvm-test-suite/MultiSource/Benchmarks/Bullet:
“Segmentation fault (core dumped)”. I've used gdb to print stack
trace:
Segmentation fault (core dumped)
ore was generated by `./bullet.simple'.
Program terminated with signal 11, Segmentation fault.
#0 upcast (colObj=0x0)
at
/home/roman/llvm-test-suite/MultiSource/Benchmarks/Bullet/include/BulletDynamics/Dynamics/btRigidBody.h:170
170 if (colObj->getInternalType()==btCollisionObject::CO_RIGID_BODY)
(gdb) bt
#0 upcast (colObj=0x0)
at
/home/roman/llvm-test-suite/MultiSource/Benchmarks/Bullet/include/BulletDynamics/Dynamics/btRigidBody.h:170
#1 saveKinematicState (timeStep=<optimized out>, this=<optimized out>)
at
/home/roman/llvm-test-suite/MultiSource/Benchmarks/Bullet/btDiscreteDynamicsWorld.cpp:113
#2 btDiscreteDynamicsWorld::stepSimulation (this=0x14a6ca0,
timeStep=0.0166666675, maxSubSteps=1, fixedTimeStep=0.0166666675)
at
/home/roman/llvm-test-suite/MultiSource/Benchmarks/Bullet/btDiscreteDynamicsWorld.cpp:333
#3 0x0000000000401ed8 in BenchmarkDemo::clientMoveAndDisplay (
this=0x7fff90475840)
at
/home/roman/llvm-test-suite/MultiSource/Benchmarks/Bullet/BenchmarkDemo.cpp:232
#4 0x0000000000401bd2 in main (argc=<optimized out>, argv=<optimized out>)
at /home/roman/llvm-test-suite/MultiSource/Benchmarks/Bullet/main.cpp:63
The executable is produced by compilation of 317 files. That's why I
think that it is be better to temporary postpone the consideration.
What do you think about this?
Is there anything else you still would like to do? Otherwise, I believe
fixing the last remaining bugs is of high importance, as we want to
enable this code as soon as possible to avoid future bitrot.
I understand that reducing this may require some work, but I don't think
it is that hard. Specifically, you could first compile the individual
*.cpp files to .o files once using once graphite once not.
You could then copy the files that are compiled without graphite into a
new directory and run a script that does the following in a loop:
prev_file = None
for file in `ls graphite/*`:
link all .o files to executable (gcc compfiles/*.o -o executable)
result = run executable (./executable)
if (result = OK)
echo File 'pref_file' introduced bug
break
cp graphite/file to ./compfiles/
prev_file = file
Cheers,
Tobias
