OP-DSL / OP2-Common

OP2: open-source framework for the execution of unstructured grid applications on clusters of GPUs or multi-core CPUs
https://op-dsl.github.io
Other
98 stars 46 forks source link

Compilation fails for repeated kernel with different arguments #109

Open spjammy opened 6 years ago

spjammy commented 6 years ago

Hi,

The Compilation fails for repeated kernel with different arguments. Sequentail version works but not genseq or other translated versions. The error is repeated par_loop kernels.

op_par_loop(test, "test1", faces, op_arg_dat( a, -1, OP_ID, 1, OP_READ), op_arg_dat( c 0, some_mapping , 1, OP_READ), op_arg_dat( c, 1 some_mapping , 1, OP_READ));

op_par_loop(test, "test1", faces, op_arg_dat( d, -1, OP_ID, 1, OP_READ), op_arg_dat( e, 0, some_mapping , 1, OP_READ), op_arg_dat( e, 1 some_mapping , 1, OP_READ));

Is it easy to handle such in the translator?

reguly commented 6 years ago

The kernel specs you wrote down are missing arguments and commas. op_par_loop(test, "test1", faces, op_arg_dat( a, -1,OP_ID, 1, "double",OP_READ), op_arg_dat( c, 0, some_mapping , 1, "double",OP_READ), op_arg_dat( c, 1, some_mapping , 1, "double",OP_READ));

op_par_loop(test, "test1", faces, op_arg_dat( d, -1,OP_ID, 1, "double",OP_READ), op_arg_dat( e, 0, some_mapping , 1, "double",OP_READ), op_arg_dat( e, 1, some_mapping , 1, "double",OP_READ));

This does work for me with the master branch. Please confirm and close the issue if it works.

spjammy commented 6 years ago

Sorry I missed the arguments double while creating the representative case.

here is the actual code // Checking internal faces op_par_loop(check_normals, "normals", faces, op_arg_dat(p_face_normal, -1, OP_ID, 2, "double", OP_RW), op_arg_dat(p_cell_centroid, 1, opface_cell_map, 2, "double", OP_READ), op_arg_dat(p_face_centre, -1, OP_ID, 2, "double", OP_READ)); //-------CHECKING BOUNDARY FACE NORMAL-----// op_par_loop(check_normals, "normals_b", bfaces, op_arg_dat(p_face_normal_b, -1, OP_ID, 2, "double", OP_RW), op_arg_dat(p_cell_centroid, 0, opboundary_cell_map, 2, "double", OP_READ),
op_arg_dat(p_face_centre_b, -1, OP_ID, 2, "double", OP_READ) ); It translates well. But fails in compilation

make solver_genseq

check_normals.h: In function ‘void check_normals(double, const double, const double)’: check_normals.h:1:13: error: redefinition of ‘void check_normals(double, const double, const double)’ inline void check_normals(double face_normals, const double cell_centroid, const double face_centre) ^ In file included from check_normals_seqkernel.cpp:6:0, from cfdsolver_seqkernels.cpp:22: check_normals.h:1:13: note: ‘void check_normals(double, const double, const double)’ previously defined here inline void check_normals(double face_normals, const double cell_centroid, const double face_centre) ^ In file included from cfdsolver_seqkernels.cpp:23:0: check_normals_seqkernel.cpp: In function ‘void op_par_loop_check_normals(const char, op_set, op_arg, op_arg, op_arg)’: check_normals_seqkernel.cpp:9:6: error: redefinition of ‘void op_par_loop_check_normals(const char, op_set, op_arg, op_arg, op_arg)’ void op_par_loop_check_normals(char const name, op_set set

reguly commented 6 years ago

Hmm, this is a use case we haven’t come across before. The mappings are just too different for these two to be compatible ü I recommend that you make a copy of the kernel check_normals as a workaround. Will try to come up with something better.

On 20 Sep 2017, at 14:59, Satya P Jammy notifications@github.com wrote:

Sorry I missed the arguments double while creating the representative case.

here is the actual code // Checking internal faces op_par_loop(check_normals, "normals", faces, op_arg_dat(p_face_normal, -1, OP_ID, 2, "double", OP_RW), op_arg_dat(p_cell_centroid, 1, opface_cell_map, 2, "double", OP_READ), op_arg_dat(p_face_centre, -1, OP_ID, 2, "double", OP_READ)); //-------CHECKING BOUNDARY FACE NORMAL-----// op_par_loop(check_normals, "normals_b", bfaces, op_arg_dat(p_face_normal_b, -1, OP_ID, 2, "double", OP_RW), op_arg_dat(p_cell_centroid, 0, opboundary_cell_map, 2, "double", OP_READ), op_arg_dat(p_face_centre_b, -1, OP_ID, 2, "double", OP_READ) ); It translates well. But fails in compilation

make solver_genseq

check_normals.h: In function ‘void check_normals(double, const double, const double)’: check_normals.h:1:13: error: redefinition of ‘void check_normals(double, const double, const double)’ inline void check_normals(double face_normals, const double cell_centroid, const double face_centre) ^ In file included from check_normals_seqkernel.cpp:6:0, from cfdsolver_seqkernels.cpp:22: check_normals.h:1:13: note: ‘void check_normals(double, const double, const double)’ previously defined here inline void check_normals(double face_normals, const double cell_centroid, const double face_centre) ^ In file included from cfdsolver_seqkernels.cpp:23:0: check_normals_seqkernel.cpp: In function ‘void op_par_loop_check_normals(const char, op_set, op_arg, op_arg, op_arg)’: check_normals_seqkernel.cpp:9:6: error: redefinition of ‘void op_par_loop_check_normals(const char, op_set, op_arg, op_arg, op_arg)’ void op_par_loop_check_normals(char const name, op_set set

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OP2/OP2-Common/issues/109#issuecomment-330843428, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNq1Sorc8PMkn8Ae-PogXDnURz80ilQks5skQwegaJpZM4PcI8q.

spjammy commented 6 years ago

Yes, I did the workaround currently by copying check_normals for the boundary loop to check_normals_b.