On Fri, 24 May 2013, Martin Jambor wrote:
> Hi,
>
> On Thu, May 23, 2013 at 11:38:10AM +0200, Richard Biener wrote:
> > On Thu, 23 May 2013, Eric Botcazou wrote:
> >
> > > > earlier this week I asked on IRC whether we could have non-top-level
> > > > BIT_FIELD_REFs and Richi said that we could. However, when I later
> > > > looked at SRA code, quite apparently it is not designed to handle
> > > > non-top-level BIT_FIELD_REFs, IMAGPART_EXPRs or REALPART_EXPRs. So in
> > > > order to test whether that assumption is OK, I added the following
> > > > into the gimple verifier and ran bootstrap and testsuite of all
> > > > languages including Ada and ObjC++ on x86_64. It survived, which
> > > > makes me wondering whether we do not want it in trunk.
> > >
> > > This looks plausible to me, but I think that you ought to verify the real
> > > assumption instead, which is that the type of the 3 nodes is always
> > > scalar.
> > > The non-toplevelness of the nodes is merely a consequence of this
> > > property.
> >
> > Yeah. But please put the verification into tree-cfg.c:verify_expr
> > instead.
> >
>
> Like this? Also bootstrapped and tested on x86_64-linux.
>
> Thanks,
>
> Martin
>
>
> 2013-05-23 Martin Jambor <mjam...@suse.cz>
>
> * tree-cfg.c (verify_expr): Verify that BIT_FIELD_REFs, IMAGPART_EXPRs
> and REALPART_EXPRs have scalar type.
>
> Index: src/gcc/tree-cfg.c
> ===================================================================
> --- src.orig/gcc/tree-cfg.c
> +++ src/gcc/tree-cfg.c
> @@ -2669,10 +2669,17 @@ verify_expr (tree *tp, int *walk_subtree
>
> case REALPART_EXPR:
> case IMAGPART_EXPR:
> + case BIT_FIELD_REF:
> + if (!is_gimple_reg_type (TREE_TYPE (t)))
> + {
> + error ("non-scalar BIT_FIELD_REF, IMAGPART_EXPR or REALPART_EXPR");
> + return t;
> + }
> + /* Fall-through. */
> case COMPONENT_REF:
> case ARRAY_REF:
> case ARRAY_RANGE_REF:
> - case BIT_FIELD_REF:
> case VIEW_CONVERT_EXPR:
> /* We have a nest of references. Verify that each of the operands
> that determine where to reference is either a constant or a variable,
Yes, that looks good to me. Note that this still does not verify
that REALPART_EXPR, IMAGPART_EXPR and BIT_FIELD_REF are only
outermost handled-component refs. It merely verifies that if they
are outermost then they are not aggregate.
Thus a followup would be to move the BIT_FIELD_REF handling in the
loop below to the above case sub-set and disallow BIT_FIELD_REF,
REALPART_EXPR and IMAGPART_EXPR inside that loop.
Though I'm pretty sure that evetually this will fail ...
The patch is ok, it's an improvement over the current state.
Thanks,
Richard.
