Ian Lance Taylor - Re: PATCH: CALL_EXPR representation part 1/9 (tree changes + documentati (original) (raw)

This is the mail archive of the gcc-patches@gcc.gnu.orgmailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

From: Ian Lance Taylor
To: Sandra Loosemore
Cc: GCC Patches , Brooks Moses , Lee Millward
Date: 10 Feb 2007 13:00:44 -0800
Subject: Re: PATCH: CALL_EXPR representation part 1/9 (tree changes + documentation)
References: <45CDE0C1.8050502@codesourcery.com>

Sandra Loosemore sandra@codesourcery.com writes:

/* In a tcc_vl_exp node, operand 0 is an INT_CST node holding the operand

length. Note that we have to bypass the use of TREE_OPERAND to access

that field to avoid infinite recursion in expanding the macros. */

#define VL_EXP_OPERAND_LENGTH(NODE) \

((int)TREE_INT_CST_LOW (VL_EXP_CHECK (NODE)->exp.operands[0]))

The comment should explicitly say that the operand length includes the length operand itself. That is, the minimum valid length is 1.

/* In ordinary expression nodes. */

#define TREE_OPERAND_LENGTH(NODE) tree_operand_length (NODE)

Do you have any timings of the compiler with your patch? When checking is enabled this is going to add a lot of function calls. I would like to see tree_operand_length be a static inline function in tree.h, unless there is some reason not to do that.

/* 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.

Please put spaces around the '+' in CALL_EXPR_ARG.

/* CALL_EXPR_ARGP returns a pointer to the argument vector for NODE.

We can't use &CALL_EXPR_ARG0 (NODE) because that will complain if

the argument count is zero when checking is enabled. Instead, do

the pointer arithmetic to advance past the 3 fixed operands in a

CALL_EXPR. That produces a valid pointer to just past the end of the

argument array, even if it's not valid to dereference it. */

#define CALL_EXPR_ARGP(NODE) \

(&(TREE_OPERAND (VL_EXP_CHECK (NODE), 0)) + 3)

This should also use CALL_EXPR_CHECK.

extern tree build_nt_call_list (enum tree_code, tree, tree);

extern tree build_call_list (enum tree_code, tree, tree, tree);

extern tree build_call_nary (enum tree_code, tree, tree, int, ...);

extern tree build_call_valist (enum tree_code, tree, tree, int, va_list);

extern tree build_call_array (enum tree_code, tree, tree, int, tree*);

The names and current implementations of these functions imply that they only build CALL_EXPRs. In that case, they don't need to take a tree_code parameter. I'd like these to be reworked so that either build general tcc_vl_exp nodes, or they just build CALL_EXPR nodes.

extern tree fold_build_call_list (enum tree_code, tree, tree, tree);

extern tree fold_build_call_list_initializer (enum tree_code, tree, tree, tree);

Here too: build a CALL_EXPR (and don't take a tree_code parameter), or build a general list (and change the name of the function).

*************** substitute_in_expr (tree exp, tree f, tr *** 2472,2477 **** --- 2502,2528 ---- } break;

```
  case tcc_vl_exp:
```
```
{
```
```
  tree copy = NULL_TREE;
```
```
  int i;
```
```
  int n = TREE_OPERAND_LENGTH (exp);
```
```
  for (i = 0; i < n; i++)
```
```
    {
```
```
      tree op = TREE_OPERAND (exp, i);
```

      tree newop = SUBSTITUTE_IN_EXPR (op, f, r);

```
      if (newop != op)
```
```
    {
```
```
      copy = copy_node (exp);
```
```
      TREE_OPERAND (copy, i) = newop;
```
```
    }
```
```
    }
```
```
  if (copy)
```
```
    new = fold (copy);
```
```
  else
```
```
    return exp;
```
```
}
```

The loop over the operands can start at 1, since operand 0 is known to be the length.

*************** substitute_placeholder_in_expr (tree exp *** 2602,2607 **** --- 2655,2682 ---- } break;

```
  case tcc_vl_exp:
```
```
{
```
```
  tree copy = NULL_TREE;
```
```
  int i;
```
```
  int n = TREE_OPERAND_LENGTH (exp);
```
```
  for (i = 0; i < n; i++)
```
```
    {
```
```
      tree op = TREE_OPERAND (exp, i);
```

      tree newop = SUBSTITUTE_PLACEHOLDER_IN_EXPR (op, obj);

```
      if (newop != op)
```
```
    {
```
```
      if (!copy)
```
```
        copy = copy_node (exp);
```
```
      TREE_OPERAND (copy, i) = newop;
```
```
    }
```
```
    }
```
```
  if (copy)
```
```
    return fold (copy);
```
```
  else
```
```
    return exp;
```
```
}
```

Here also.

*************** build_omp_clause (enum omp_clause_code c *** 7305,7310 **** --- 7396,7554 ---- return t; }
/* Set various status flags when building a tcc_vl_exp object T. */

static void

process_call_operands (tree t)

{

bool side_effects;

side_effects = TREE_SIDE_EFFECTS (t);

if (!side_effects)
{
  int i, n;
  n = TREE_OPERAND_LENGTH (t);
  for (i = 0; i < n; i++)
{
  tree op = TREE_OPERAND (t, i);
  if (op && TREE_SIDE_EFFECTS (op))
    {
      side_effects = 1;
      break;
    }
}

Here also.

}
if (TREE_CODE (t) == CALL_EXPR && !side_effects)
{
  int i;
  /* Calls have side-effects, except those to const or
 pure functions.  */
  i = call_expr_flags (t);
  if (!(i & (ECF_CONST | ECF_PURE)))
side_effects = 1;
}
TREE_SIDE_EFFECTS (t) = side_effects;

}

This function is named process_call_operands, so you don't need to check for CALL_EXPR. Or, if you want to handle other node types either, you should give the function a different name.

/* Build a tcc_vl_exp object with code CODE and room for LEN operands. LEN

includes the implicit operand count in TREE_OPERAND 0, and so must be >= 1.

Except for the CODE and operand count field, other storage for the

object is initialized to zeros. */

tree

build_vl_exp_stat (enum tree_code code, int len MEM_STAT_DECL)

{

tree t;

int length = (len-1) * sizeof (tree) + sizeof (struct tree_exp);

Please add spaces around the '-'.

/* Build and return a TREE_LIST of arguments in the CALL_EXPR exp.

FIXME: don't use this function. It exists for compatibility with

the old representation of CALL_EXPRs where a list was used to hold the

arguments. Places that currently extract the arglist from a CALL_EXPR

ought to be rewritten to use the CALL_EXPR itself. */

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.

Sorry for all the requested changes. I think they should be fairly mechanical. Send another version of this patch when they are done.

Thanks.

Ian

Follow-Ups:
- Re: PATCH: CALL_EXPR representation part 1/9 (tree changes + documentation)
  * From: Sandra Loosemore
References:
- PATCH: CALL_EXPR representation part 1/9 (tree changes + documentation)
  * From: Sandra Loosemore

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]