On Mon, 11 May 2015, Tom de Vries wrote:
> On 11-05-15 09:47, Richard Biener wrote:
> > > Bootstrapped and reg-tested on x86_64, with and without -m32.
> > > >
> > > >OK for trunk?
> > > >
> > > >[ FWIW, I suspect this patch will make life easier for the
> > > reimplementation of
> > > >the pass_stdarg optimization using ifn_va_arg. ]
> > + if (canon_va_type != NULL)
> > + {
> > + if (!(TREE_CODE (canon_va_type) == ARRAY_TYPE
> > + && TREE_CODE (va_type) != ARRAY_TYPE))
> > + /* In gimplify_va_arg_expr we take the address of the ap argument,
> > mark
> > + it addressable now. */
> > + mark_addressable (expr);
> >
> > can we "simplify" this and ...
> >
> > - }
> > -
> > + gcc_assert (TREE_CODE (TREE_TYPE (valist)) != ARRAY_TYPE);
> > gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue);
> >
> > this to use [!]POINTER_TYPE_P ()?
>
> I'm not sure.
>
> > Why do we arrive with non-array
> > va_type but array canon_va_type in build_va_arg?
>
> grokdeclarator in c-decl.c:
> ...
> /* A parameter declared as an array of T is really a pointer to T.
> One declared as a function is really a pointer to a function. */
>
> if (TREE_CODE (type) == ARRAY_TYPE)
> {
> /* Transfer const-ness of array into that of type pointed to. */
> type = TREE_TYPE (type);
> if (type_quals)
> type = c_build_qualified_type (type, type_quals);
> type = c_build_pointer_type (type);
> ...
>
> > I suppose the
> > va_list argument already decayed to a pointer then
>
> The above means that the va_list function parameter decayed to a pointer.
> AFAIU, the va_list argument to va_arg just uses the same type (for parsing,
> grep for RID_VA_ARG in c-parser.c).
>
> > (in which case
> > the argument should already be addressable?)?
> >
>
> The argument is of pointer type. That pointer-typed-argument will only be
> addressable if we take the address of it, which is precisely the thing we're
> trying to avoid in this patch.
>
> > I think the overall idea of the patch is good - I'm just worried about
> > special-casing of ARRAY_TYPE vs. non-pointer-type (because it's the
> > latter that we ultimately want...).
>
> AFAIU, the special casing of ARRAY_TYPE in the patch is a consequence of the
> special-casing of ARRAY_TYPE as a parameter.
>
> I don't see how [!]POINTER_TYPE_P () can help here. I've rewritten and
> attached the build_va_arg bit using POINTER_TYPE_P and expanded comments
> a bit to demonstrate.
Ah, ok.
The patch is ok.
Thanks,
Richard.
