This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix TYPE_TYPELESS_STORAGE handling (PR middle-end/80423)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Jason Merrill <jason at redhat dot com>,gcc-patches at gcc dot gnu dot org
- Date: Wed, 19 Apr 2017 09:59:32 +0200
- Subject: Re: [PATCH] Fix TYPE_TYPELESS_STORAGE handling (PR middle-end/80423)
- Authentication-results: sourceware.org; auth=none
- References: <20170418151430.GZ1809@tucnak> <23D6ECD4-AA6B-4139-8E16-C9142B90B66D@suse.de> <20170419055630.GC1809@tucnak>
On April 19, 2017 7:56:30 AM GMT+02:00, Jakub Jelinek <jakub@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