Re: VTA RTL patch review?

[Richard Henderson <rth at redhat dot com>]

On 07/23/2009 12:56 PM, Alexandre Oliva wrote:
> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg00256.html

Reviewing only vta-patchset-rtl-base.patch in this message.

> +  /* INSN_UID for next debug insn emitted.  Only used if
> +     -fmin-debug-insn-uid=<value> is given with nonzero value.  */
> +  int x_cur_debug_insn_uid;

Update comment to match change to params.

>  emit_insn_after (rtx pattern, rtx after)
>  {
> -  if (INSN_P (after))
> -    return emit_insn_after_setloc (pattern, after, INSN_LOCATOR (after));
> +  rtx prev = after;
> +
> +  while (DEBUG_INSN_P (prev))
> +    prev = PREV_INSN (prev);
> +
> +  if (INSN_P (prev))
....
>  emit_insn_before (rtx pattern, rtx before)
>  {
> -  if (INSN_P (before))
> -    return emit_insn_before_setloc (pattern, before, INSN_LOCATOR (before));
> +  rtx next = before;
> +
> +  while (DEBUG_INSN_P (next))
> +    next = PREV_INSN (next);
> +
> +  if (INSN_P (next))

Do debug insns "belong" to the preceeding or following real insn?
I would have thought insert-after would search backward and
insert-before would not search, or vice-versa, rather than
this situation in which both inserts search backward.

It might be nice to have helper functions like

static inline rtx
adjust_after_position_for_debug (rtx prev)
{
  ...
  return prev;
}

and use that in the 5 places that seem to be required.


r~