On 28/06/2012 09:34, Tobias Burnus wrote: > This patch generates inline code for C_F_POINTER with an array argument. > One reason is that GCC didn't handle SHAPE= arguments which were > noncontiguous. > > However, the real motivation is the fortran-dev branch with the new > array-descriptor: C_F_POINTER needs then to set the stride multiplier, > but as it doesn't know the size of a single element, one had either to > pass the value or handle it partially in the front end. Hence, doing it > all in the front-end was simpler. The C_F_Pointer issue is the main > cause for failing test cases on the branch, though several other issues > remain. > > Build and regtested on x86-64-linux- > OK for the trunk? >
Two comments about your patch below. > > PPS: The offset handling in gfortran is really complicated. I wonder > whether we have to (or at least should) change it for the new array > descriptor. I don't know exactly what you mean by "really complicated". There are not many simple things in gfortran ;-). And I don't know what you mean by "change it". Generally I'm in favour of changing as few things as possible, or at least changing them incrementally, but there could well exist some cases where changing everything together is just simpler. > + > + stride = gfc_create_var (gfc_array_index_type, "stride"); > + offset = gfc_create_var (gfc_array_index_type, "offset"); > + gfc_add_modify (&block, stride, gfc_index_one_node); > + gfc_add_modify (&block, offset, gfc_index_one_node); > + You initialize offset to 1... [...] > + > + /* Calculate offset. */ > + gfc_init_block (&ifblock); > + gfc_add_modify (&ifblock, offset, > + fold_build2_loc (input_location, PLUS_EXPR, > + gfc_array_index_type, offset, stride)); > + tmp = fold_build2_loc (input_location, GT_EXPR, boolean_type_node, > + loop.loopvar[0],loop.from[0]); > + tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, tmp, > + gfc_finish_block (&ifblock), > + build_empty_stmt (input_location)); > + gfc_add_expr_to_block (&body, tmp); > + ...and then update it in the loop with an if condition to skip the first iteration. Wouldn't it be better to initialize to 0, and update without condition? [...] > + > + gfc_add_modify (&block, offset, > + fold_build2_loc (input_location, MULT_EXPR, > + gfc_array_index_type, offset, > + build_int_cst (gfc_array_index_type, > + -1))); Could/Should we use a NEGATE_EXPR instead of a multiplication by -1? Patch pre-approved with the two nits above fixed. Thanks. Mikael
