Hi Julian, LGTM. Thanks!
* * * Can you as follow-up also add a testcase which uses – instead of integer – 'character' (with len > 1, both kind=1 and kind=4) and check that it is handled correctly? In particular, that the right size is passed on. (Should be size-in-bytes = kind*len.) It probably just work, unless any OpenACC construct uses a different code path than the one covered by my OpenMP patches (esp. https://gcc.gnu.org/g:102502e32ea4e8a75d6b252ba319d09d735d9aa7 ). ('character' (esp. kind=4 or len=: (¹)) and 'class' have the tendency to break. Not that allocatable/pointer/optional don't expose bugs.) Tobias (¹) Bug regarding (len=:) in components ishttps://gcc.gnu.org/PR95868. On 04.02.21 14:58, Julian Brown wrote:
Thanks for review! Comments below. [...] I think the code was correct wrt. data sizes, but the logic was admittedly rather convoluted. I think the attached is better -- the essential "weirdness" of the previous patch is that it seemed like a BT_DERIVED-typed "inner" could either be a pointer or not, and likewise for the BT_CLASS case. Actually though, that wasn't true. All the non-POINTER_TYPE_P cases involve BT_DERIVED, because the decl is transparently dereferenced by the earlier gfc_conv_component_ref call. BT_CLASS pointers, however, were not dereferenced -- so "data" appeared as an actual pointer. The pre-patch code actually only worked in the BT_CLASS case. So, I think the new version makes more sense. (The "size" field is either transparently dereferenced via gfc_conv_component_ref, or comes from the class's vtable, so it's never the size of the non-dereferenced pointer.) Re-tested with offloading to AMD GCN. OK? Thank you, Julian
----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
