On Mon, 14 Sep 2015, Alan Lawrence wrote: > Ping. (Rerevert with 5 lines extra paranoia in scalarizable_type_p).
Sorry for chiming in so late... + if (TYPE_DOMAIN (type) == NULL_TREE + || !TREE_CONSTANT (TYPE_MIN_VALUE (TYPE_DOMAIN (type))) + || !TREE_CONSTANT (TYPE_MAX_VALUE (TYPE_DOMAIN (type))) + || !TREE_CONSTANT (TYPE_SIZE (type)) + || (tree_to_shwi (TYPE_SIZE (type)) <= 0)) + return false; TREE_CONSTANT isn't the correct thing to test. You should use TREE_CODE () == INTEGER_CST instead. Also you need to handle NULL_TREE TYPE_MIN/MAX_VALUE as that can happen as well. + if (DECL_P (elem) && DECL_BIT_FIELD (elem)) + return false; that can't happen (TREE_TYPE (array-type) is never a DECL). + int el_size = tree_to_uhwi (elem_size); + gcc_assert (el_size); so you assert on el_size being > 0 later but above you test only array size ... + tree t_idx = build_int_cst (TYPE_DOMAIN (decl_type), idx); use t_idx = size_int (idx); + tree nref = build4 (ARRAY_REF, elemtype, ref, t_idx, NULL_TREE, + NULL_TREE); Otherwise looks ok to me. Thanks, Richard. > Thanks, Alan > > On 08/09/15 13:43, Martin Jambor wrote: > > Hi, > > > > On Mon, Sep 07, 2015 at 02:15:45PM +0100, Alan Lawrence wrote: > > > In-Reply-To: <55e0697d.2010...@arm.com> > > > > > > On 28/08/15 16:08, Alan Lawrence wrote: > > > > Alan Lawrence wrote: > > > > > > > > > > Right. I think VLA's are the problem with pr64312.C also. I'm testing > > > > > a fix > > > > > (that declares arrays with any of these properties as unscalarizable). > > > > ... > > > > In the meantime I've reverted the patch pending further testing on x86, > > > > aarch64 > > > > and arm. > > > > > > I've now tested g++ and fortran (+ bootstrap + check-gcc) on x86, AArch64 > > > and > > > ARM, and Ada on x86 and ARM. > > > > > > So far the list of failures from the original patch seems to be: > > > > > > * g++.dg/torture/pr64312.C on ARM and m68k-linux > > > * Building Ada on x86 > > > * Ada ACATS c87b31a on ARM (where the Ada frontend builds fine) > > > > > > Here's a new version, that fixes all the above, by adding a dose of > > > paranoia in scalarizable_type_p... > > > > I have only had a bref look at scalarizable_type_p then, considering > > all of the rest unchanged, and the tests there seem natural to me. > > (Note that I do not have the authority to approve the patch.) > > > > > (I wonder about adding a comment > > > in completely_scalarize that such cases have already been ruled > > > out?) > > > > The comment already references scalarizable_type_p which is enough at > > least for me. > > > > Thanks, > > > > Martin > > > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)