(CC list trimmed.)
On Wed, 31 Oct 2012, Kirill Yukhin wrote:
> Hi,
> This patch introduces a new RTL expression called define_subst and
> required by it define_subst_attr.
>
> The new feature allows to make MD-files more compact - it defines a
> rule by which a parser could generate modified versions of
> RTL-templates. For example, i386.md contains RTL-templates for
> ordinary arithmetic, as well as templates for arithmetic with
> zero-extension. With define_subst one could specify one rule (write
> one define_subst) and keep only RTL-templates for ordinary arithmetic
> - the versions with zero-extensions will be generated automatically.
> The rule in this case would describe how zero-extended pattern could
> be generated with given pattern without zero-extension.
>
> There are some more examples in existing RTL templates where
> define_subst could be useful, and it would be extremely useful for
> future x86 ISA extensions.
>
> I'd be glad to have a feedback on this feature and ask any questions.
Sounds great, (I think) this is something I've longed to see
ever since I saw the iterator machinery (thanks, Richard S!) and
wanted to do structural replacements just as easily.
But... I don't really understand it, so here's some feedback on
the documentation: Regarding the language, a definite article is
missing in about half the places where one is needed, at least
it seems so to me; not a native English speaker. There are
other similar grammar issues. I suggest having a native speaker
proofread it. (I'm just guessing that being with Intel gives
the benefit that there's someone that could do that for you.)
I think it'd make a big difference if you put the example first
and the detailed explanation *after that*, around "Most of
expressions" (or as it should be: "Most expressions").
Start with a pattern where the feature is used as intended in
the .md and show the expansion only *after* that; it seems
you're doing it the other way round (or I'm confused). Try to
avoid using other types of iterator expansions in the same
example. To do that, it's ok to simplify the example: it
doesn't have to be exactly as actually used; it likely won't
stay that way for long anyway.
Specifically, I don't get the "If @code{define_subst} could not
be applied because the pattern does not match the
@code{define_insn}/@code{define_expand} original template, then
the second copy is deleted." I think an example showing such an
expansion (partial and failed) would do wonders.
Thanks for your work, I hope something like this gets in for 4.8.
brgds, H-P
