[PATCH 2/5] completely_scalarize arrays as well as records.

Richard Biener rguenther@suse.de
Tue Sep 15 07:49:00 GMT 2015


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.2010008@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)



More information about the Gcc-patches mailing list