On April 19, 2017 7:56:30 AM GMT+02:00, Jakub Jelinek <ja...@redhat.com> wrote: >On Wed, Apr 19, 2017 at 07:45:36AM +0200, Richard Biener wrote: >> >As mentioned in the PR, we now use TYPE_TYPELESS_STORAGE flag on >> >ARRAY_TYPEs to denote types that need the special C++ alias >handling. >> >The problem is how is that created, we just use build_array_type and >> >set TYPE_TYPELESS_STORAGE on the result, but build_array_type uses >type >> >caching, so we might modify that way some other array type. >> >If all the array type creation goes through build_cplus_array_type, >> >that >> >wouldn't be a problem, as that flag is dependent just on the element >> >type, but that is not the case, c-family as well as the middle-end >has >> >lots of spots that also create array types. So in the end whether >> >one gets TYPE_TYPELESS_STORAGE flag or not is quite random, depends >on >> >GC etc. >> > >> >The following patch attempts to resolve this, by making the type >> >hashing >> >take that flag into account. Bootstrapped/regtested on x86_64-linux >> >and >> >i686-linux, ok for trunk? >> >> When changing the C++ function I thought that calling >build_array_type was >> wrong and it should instead do the same it does in the other places, >use >> its raw creation routine and then the canonical type register stuff. >But >> I was hesitant to change this at this point. > >The problem is that as the patch shows, we don't need it in a single >place >(the C++ FE), but at least in two places (C++ FE and c-family), and it >wouldn't surprise me if we don't need it later on in further places >(e.g. in middle-end, if we have a TYPE_TYPELESS_STORAGE array and say >DSE >wants to create a smaller one with the same property).
Hmm, but the C++ FE largely avoids the type cache of the Me and uses its own. Apart from a single place in this function. But as I said I'm AFK and can't really check. >Using a default argument to build_array_type is likely cleaner indeed, >I'd just then also swap the arguments to build_array_type_1 (the shared >vs. typeless_storage). Sure. With C++ there's no reason to export _1 variants. Either overloading or default args work for me here. Richard. > Jakub
