Re: PATCH RFA: Permit macro attributes to set modes

[Richard Sandiford <rsandifo at redhat dot com>]

Hi Ian,

Sorry for not responding earlier.

Ian Lance Taylor <ian@airs.com> writes:
> I'm working on a backend which supports two different sizes of
> vectors.  There are some instructions which operate on both vector
> sizes.  Either vector may have elements of type int, short, etc., but
> naturally both vectors need to have the same subtype.
>
> Now that we have .md file macros, it would sure be nice to be able to
> use them to write insn patterns in which setting one of the modes
> determined the other modes.  That would let me write a single pattern
> which used vectors of different sizes, and used a macro to iterate
> over all the possible element types.  For example, VD is a macro for
> all the double vector modes.  I would like to be able to define an
> attribute VSMODE for the corresponding single vector modes, and then
> write:
>
>   (set (match_operand:VD 0 "register_operand" "=v")
>        (vec_concat:VD (match_operand:<VSMODE> 1 "register_operand" "v")
>                       (match_operand:<VSMODE> 2 "register_operand" "v")))

Very nice, thanks!

My one concern is about the checking code.  We don't complain about
unrecognised "<...>" sequences in strings because there's no guarantee
that a particular "<" or ">" character was supposed to be part of a
string substitution.  That doesn't apply to modes though.  It should
be a hard error if "<FOO:BARMODE>" doesn't match anything.

As it stands, it looks like you check for recognised attribute
names when the mode is first parsed:

> +  /* Make sure the attribute is defined as either a code attribute or
> +     a mode attribute.  */
> +  attr = strchr (p, ':');
> +  if (attr == 0)
> +    attr = p;
> +  else
> +    ++attr;
> +
> +  if (!htab_find (modes.attrs, &attr) && !htab_find (codes.attrs, &attr))
> +    fatal_with_file_and_line (infile,
> +			      "undefined attribute '%s' used for mode",
> +			      p);

But even if this check succeeds, the mode will not be replaced if either:

  (a) there is a typo in the "FOO" part of "<FOO:BARMODE>" or
  (b) attribute BARMODE doesn't have an entry for a particular
      member of FOO.

Thus we could still have an rtx with an invalid mode, which would cause
surprises later on.

I wonder if it would be better to remove the checks above and instead
check for unreplaced modes after expansion.  Perhaps one way of doing
this would be to add a field like:

  struct map_value *unexpanded_mode;

to your macro_traverse_data structure.  This would be set to the
first mode_maps entry before applying any macros:
     
      mtd.queue = queue_next;
      mtd.mode_maps = mode_maps;
-->   mtd.unexpanded_mode = mode_maps;
      mtd.infile = infile;
      htab_traverse (modes.macros, apply_macro_traverse, &mtd);
      htab_traverse (codes.macros, apply_macro_traverse, &mtd);

set to NULL before applying a particular macro:

  for (elem = (rtx) data; elem != 0; elem = XEXP (elem, 1))
    if (uses_macro_p (XEXP (elem, 0), macro))
      {
-->     mtd->unexpanded_mode = NULL;

and then set whenever the map_attr_string() call in apply_mode_maps() fails:

	  v = map_attr_string (pm->string, macro, value);
	  if (v)
	    {
	      PUT_MODE (x, find_mode (v->string, infile));
	      return;
	    }
-->       mtd->unexpanded_macro = pm;

(where mtd would obviously have to be passed to apply_mode_maps()).
We could then just check mtd->unexpanded macro after all macros
have been expanded.

Richard