PATCH: CALL_EXPR representation part 1/9 (tree changes + documentation)

[Steven Bosscher]

On Sunday 11 February 2007 00:28, Sandra Loosemore wrote:
> >> + /* CALL_EXPR accessors.
> >> +  */
> >> + #define CALL_EXPR_FN(NODE) TREE_OPERAND (VL_EXP_CHECK (NODE), 1)
> >> + #define CALL_EXPR_STATIC_CHAIN(NODE) TREE_OPERAND (VL_EXP_CHECK
> >> (NODE), 2) + #define CALL_EXPR_ARGS(NODE) call_expr_arglist (NODE)
> >> + #define CALL_EXPR_ARG0(NODE) TREE_OPERAND (VL_EXP_CHECK (NODE), 3)
> >> + #define CALL_EXPR_ARG1(NODE) TREE_OPERAND (VL_EXP_CHECK (NODE), 4)
> >> + #define CALL_EXPR_ARG2(NODE) TREE_OPERAND (VL_EXP_CHECK (NODE), 5)
> >> + #define CALL_EXPR_ARG(NODE, I) TREE_OPERAND (VL_EXP_CHECK (NODE),
> >> (I)+3) + #define call_expr_nargs(NODE) (VL_EXP_OPERAND_LENGTH(NODE) - 3)
> >
> > These macros should all check that they are being applied to a
> > CALL_EXPR node.  Please change these to use the existing
> > CALL_EXPR_CHECK macro.  We may have other variable length tree codes
> > in the future, so checking VL_EXP_CHECK is not what we need here.
>
> Unfortunately, your suggestion will break the C++ front end, which defines
> AGGR_INIT_EXPR to be a tcc_vl_exp with the same representation as
> CALL_EXPR, and in some places has "generic" code that operates on both
> kinds of objects.

That sounds wrong.  If these trees do not have the same TREE_CODE, they
should also not be using the same accessor macros.  You should give the
AGGR_INIT_EXPR tree its own accessor macros.

More work, yes.  But the only proper design, I'd say.
 

> Maybe this is just bad design in the C++ front end, and there shouldn't be
> any such CALL_EXPR-like things or dependence on generic code.  OTOH, the
> purpose of this patch isn't to re-engineer the C++ front end.

But its purpose is to re-engineer CALL_EXPRs, and tcc_vl_exp is a new
kind of tree node that your patch introduces.  IMHO it's only wise that
you would avoid the mistakes of the past while you're re-engineering
the affected code.


> >> + tree
> >> + call_expr_arglist (tree exp)
> >> + {
> >> +   tree arglist = NULL_TREE;
> >> +   int i;
> >> +   for (i = call_expr_nargs (exp) - 1; i >= 0; i--)
> >> +     arglist = tree_cons (NULL_TREE, CALL_EXPR_ARG (exp, i), arglist);
> >> +   return arglist;
> >> + }
> >
> > Do you still need this function?  It's fine if you do need it.
>
> Yes, unfortunately there are still a few places that use it.  They can (and
> should) be rewritten, but it would involve a lot of code restructuring
> and/or things we couldn't test easily (config/frv and config/mips, for
> instance).

Can you promise that someone will do this before gcc 4.3 goes out?  GCC
has already had way too many half completed restructuring projects.  
Let's please not add more nearly-complete reworks, because there usually
is no-one around to add the finishing touch.

Both MIPS and FRV have working simulators AFAIK, so you should be able
to test your changes.

Gr.
Steven