Mikael Morin wrote:
The current API touching array descriptors is mostly limited to a set of getters to pick the value of individual array descriptor fields, and setters to update them. This requires every area of the compiler touching array descriptors to have knowledge of their contents.
In terms of the 2nd part of the series, I think the following shows the benefit for moving to setter/getter functions (from 02/11 of part 2): - tmp = gfc_conv_descriptor_dtype (tmp); - field = gfc_advance_chain (TYPE_FIELDS (get_dtype_type_node ()), - GFC_DTYPE_ELEM_LEN); - tmp = fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (field), - tmp, field, NULL_TREE); + tmp = gfc_conv_descriptor_elem_len_get (tmp); Admittedly, that cleanup could have also be done with the current scheme and without a '_get' function as other changes in this patch show. - elemsz = gfc_conv_descriptor_elem_len (decl); + elemsz = gfc_conv_descriptor_elem_len_get (decl); but still, it was done as part of this work. * * * The longer term benefit is of course to handle extend vs. ubound and possibly also the offset calculation better – at least if we want to move to a stride multiplier (in bytes, sm) instead of stride (in multiples of elem_len) eventually. I also find the current offset + lbound/ubound calculations confusing and if they can be moved into something more readable or more contained, it is surely useful. * * * Only slightly related as there is elem_len, but one area were we need to improve is also the check of the unit size of some objects, especially for strings where we have to deal with KIND, elem_len, polymorphism etc. There is a lot of code where which breaks - by either ignoring the kind value or using TREE_UNIT_SIZE or missing the right size field with polymorphism or array descriptors etc. I with a helper function would be useful - but admittedly, it is only related to array descriptors and not really part of this series.
The final goal of the patches is to remove (most of) the setters from the public API, and replace them with more high level functions providing complete array descriptor updates (say copy descriptor, or shift array bounds) without leaking the detail of the fields that have their values changed under the hood. This first series moves the existing array descriptor functions to a separate file. Mikael Morin (7): fortran: array descriptor: Move accessor functions to a separate file fortran: array descriptor: Move debug info generation function fortran: array descriptor: Move null factory function fortran: array descriptor: Move size and cosize functions fortran: array descriptor: Move bound shift utility function fortran: array descriptor: Move descriptor copy function fortran: array descriptor: Move array growth function
The 7 patches of this patchset LGTM. Steve also wrote on November 11:
This series of patches is OK with me. After applying each patch, I bootstrapped and ran check-fortran on x86_64-*-freebsd. Everything worked as expected, i.e., no regressions. Thanks for taking on these clean-up type of patches.
Thanks, Tobias
