Hi, On Wed, Aug 26, 2015 at 09:07:33AM +0200, Richard Biener wrote: > On Tue, Aug 25, 2015 at 11:44 PM, Jeff Law <l...@redhat.com> wrote: > > On 08/25/2015 03:42 PM, Martin Jambor wrote: > >> > >> Hi, > >> > >> On Tue, Aug 25, 2015 at 12:06:14PM +0100, Alan Lawrence wrote: > >>> > >>> This changes the completely_scalarize_record path to also work on arrays > >>> (thus > >>> allowing records containing arrays, etc.). This just required extending > >>> the > >>> existing type_consists_of_records_p and completely_scalarize_record > >>> methods > >>> to handle things of ARRAY_TYPE as well as RECORD_TYPE. Hence, I renamed > >>> both > >>> methods so as not to mention 'record'. > >> > >> > >> thanks for working on this. I see Jeff has already approved the > >> patch, but I have two comments nevertheless. First, I would be much > >> happier if you added a proper comment to scalarize_elem function which > >> you forgot completely. The name is not very descriptive and it has > >> quite few parameters too. > > > > Right. I mentioned that I missed the lack of function comments when looking > > at #3 and asked Alan to go back and fix them in #1 and #2. > > > >> > >> Second, this patch should also fix PR 67283. It would be great if you > >> could verify that and add it to the changelog when committing if that > >> is indeed the case. > > > > Excellent. Yes, definitely mention the BZ. > > One extra question is does the way we limit total scalarization work well > for arrays? I suppose we have either sth like the maximum size of an > aggregate we scalarize or the maximum number of component accesses > we create? >
Only the former and that would be kept intact. It is in fact visible in the context of the last hunk of the patch. Martin
