public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <rdsandiford@googlemail.com>
To: James Greenhalgh <james.greenhalgh@arm.com>
Cc: gcc-patches@gcc.gnu.org,  gerald@pfeifer.com, 	joseph@codesourcery.com
Subject: Re: [Patch docs 3/5] Update "RTL Template" in md.texi
Date: Mon, 12 Jan 2015 22:11:00 -0000	[thread overview]
Message-ID: <871tmzac9e.fsf@googlemail.com> (raw)
In-Reply-To: <1420543302-11008-4-git-send-email-james.greenhalgh@arm.com>	(James Greenhalgh's message of "Tue, 6 Jan 2015 11:21:40 +0000")

Thanks for tackling this.

[Sorry, just realised Jeff is going through this ATM too, so sorry for
any dups]

I think Sandra and probably others prefer to avoid an explicit future
tense, e.g. "X is substituted" rather than "X will be substituted".

James Greenhalgh <james.greenhalgh@arm.com> writes:
> @@ -286,15 +284,15 @@ used only in @code{match_dup} expressions have higher values than all
>  other operand numbers.
>  
>  @var{predicate} is a string that is the name of a function that
> -accepts two arguments, an expression and a machine mode.
> -@xref{Predicates}.  During matching, the function will be called with
> -the putative operand as the expression and @var{m} as the mode
> -argument (if @var{m} is not specified, @code{VOIDmode} will be used,
> +accepts two arguments, an expression and a machine mode
> +(@pxref{Predicates}).  During matching, the function will be called
> +with the operand as the expression and @var{m} as the mode
> +argument (if @var{m} is not specified, then @code{VOIDmode} will be used,
>  which normally causes @var{predicate} to accept any mode).  If it
>  returns zero, this instruction pattern fails to match.
> -@var{predicate} may be an empty string; then it means no test is to be
> -done on the operand, so anything which occurs in this position is
> -valid.
> +
> +@var{predicate} may be an empty string.  This represents a predicate for
> +which any operand will be considered valid.

In some ways I think the original version is more accurate.
Something like (match_operand:BLK 1 "") doesn't test anything;
the :BLK is ignored.

> @@ -302,80 +300,89 @@ not always.  For example, the predicate @code{address_operand} uses
>  Many predicates accept @code{const_int} nodes even though their mode is
>  @code{VOIDmode}.
>  
> -@var{constraint} controls reloading and the choice of the best register
> -class to use for a value, as explained later (@pxref{Constraints}).
> -If the constraint would be an empty string, it can be omitted.
> -
> -People are often unclear on the difference between the constraint and the
> -predicate.  The predicate helps decide whether a given insn matches the
> -pattern.  The constraint plays no role in this decision; instead, it
> -controls various decisions in the case of an insn which does match.
> +@var{constraint} controls the best register class to use for a value,
> +and therefore register allocation and reloading, as explained
> +later (@pxref{Constraints}).  If the constraint would be an empty
> +string, it can be omitted.
> +
> +In summary, the predicate is used to control whether the instruction
> +pattern is a valid match for an insn.  The constraint is used to control
> +register allocation decisions in the case of an instruction pattern which
> +has already been matched to an insn.
> +
> +It is an error for the contraints of an operand to be impossible to fulfil
> +for operands which are valid for the predicate of the operand.  Formally;
> +for all operands for which the predicate would be true, there must exist
> +at least one valid register class for that operand.

This could be a bit misleading.  For memories it's sufficient to provide
a suitably general memory constraint (i.e. one that accepts at least
(mem (reg))).  The constraints don't also need to specify a register class
for the operand.

>  Note that this
> +restriction does not forbid accepting operands which will need additional
> +handling to move them to a valid register class.  This restriction would,
> +however, prevent combining a constraint set requiring the use of an
> +immediate value with a predicate allowing any operand, as it is not
> +possible to convert all operand types to immediate values.

The other way is also important, and in some ways easier to describe:
it is invalid for a constraint to accept something that the predicate
doesn't.  I think it might be worth saying that first.

>  @findex match_scratch
>  @item (match_scratch:@var{m} @var{n} @var{constraint})
> -This expression is also a placeholder for operand number @var{n}
> -and indicates that operand must be a @code{scratch} or @code{reg}
> +This expression is a placeholder for operand number @var{n}
> +and indicates that the operand must be a @code{scratch} or @code{reg}
>  expression.
>  
> -When matching patterns, this is equivalent to
> +When matching instruction patterns, this is equivalent to:
>  
>  @smallexample
> -(match_operand:@var{m} @var{n} "scratch_operand" @var{constraint})
> +(match_operand:@var{m} @var{n} "scratch_operand" @var{pred})
>  @end smallexample

The original was correct here.  The string is a constraint rather
than a predicate.

> -When matching an expression, it matches an expression if the function
> -@var{predicate} returns nonzero on that expression @emph{and} the
> -patterns @var{operands} match the operands of the expression.
> +When matching an insn, it matches if @var{predicate} returns nonzero for
> +the expression code @emph{and} the @var{operands} of the expression are
> +valid for the instruction pattern.

"expression" rather than "expression code" was correct here.
The predicate gets passed the rtx corresponding to operand n
and is allowed to examine its operands as well as its code.

> -Suppose that the function @code{commutative_operator} is defined as
> -follows, to match any expression whose operator is one of the
> -commutative arithmetic operators of RTL and whose mode is @var{mode}:
> +In this example, the function @code{commutative_operator} is defined to
> +match any expression whose operator is one of the commutative arithmetic
> +operators of RTL and whose mode is @var{mode}:
>  
>  @smallexample
>  int
> -commutative_integer_operator (x, mode)
> -     rtx x;
> -     machine_mode mode;
> +commutative_integer_operator (rtx x, machine_mode mode)
>  @{
>    enum rtx_code code = GET_CODE (x);
>    if (GET_MODE (x) != mode)

Sorry for the mission creep, but while you're there, it'd be good
to make the example use define_predicate instead.

> @@ -416,24 +424,29 @@ gen-function.  The subexpressions of argument 3 are not used;
>  only its expression code matters.
>  
>  When @code{match_operator} is used in a pattern for matching an insn,
> -it usually best if the operand number of the @code{match_operator}
> -is higher than that of the actual operands of the insn.  This improves
> -register allocation because the register allocator often looks at
> -operands 1 and 2 of insns to see if it can do register tying.
> -
> -There is no way to specify constraints in @code{match_operator}.  The
> -operand of the insn which corresponds to the @code{match_operator}
> -never has any constraints because it is never reloaded as a whole.
> -However, if parts of its @var{operands} are matched by
> -@code{match_operand} patterns, those parts may have constraints of
> -their own.
> +it is usually best if the operand number of the @code{match_operator}
> +is higher than that of the actual operands of the insn.  This can
> +improve register allocation because the register allocator often
> +looks at operands 1 and 2 of insns to see if they are suitable
> +for register tying.

I think we can drop this bit.  It doesn't apply to either IRA or LRA
as far as I know.

Thanks,
Richard

  parent reply	other threads:[~2015-01-12 22:02 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-06 11:21 [Patch docs 0/5] Update some of md.texi James Greenhalgh
2015-01-06 11:22 ` [Patch docs 1/5] Update the first section " James Greenhalgh
2015-01-06 16:56   ` James Greenhalgh
2015-01-06 18:55     ` Joseph Myers
2015-01-08 21:43       ` Jeff Law
2015-01-09  2:11         ` Segher Boessenkool
2015-01-12 22:29   ` Richard Sandiford
2015-01-06 11:22 ` [Patch docs 4/5] Update "Output Template/Statement" from md.texi James Greenhalgh
2015-01-12 21:46   ` Jeff Law
2015-01-12 22:19   ` Richard Sandiford
2015-01-06 11:22 ` [Patch docs 3/5] Update "RTL Template" in md.texi James Greenhalgh
2015-01-12 22:04   ` Jeff Law
2015-01-12 22:11   ` Richard Sandiford [this message]
2015-01-06 11:22 ` [Patch docs 2/5] Update "Instruction Patterns" " James Greenhalgh
2015-01-08 22:00   ` Jeff Law
2015-01-09  2:19     ` Segher Boessenkool
2015-01-11 17:40     ` James Greenhalgh
2015-01-12 22:31   ` Richard Sandiford
2015-01-06 11:23 ` [Patch docs 5/5] Update "Predicates" from md.texi James Greenhalgh
2015-01-12 22:42   ` Richard Sandiford
2015-01-06 14:57 ` [Patch docs 0/5] Update some of md.texi Joseph Myers
2015-01-06 15:26   ` James Greenhalgh
2015-01-06 17:31     ` Sandra Loosemore
2015-01-06 18:46     ` Joseph Myers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871tmzac9e.fsf@googlemail.com \
    --to=rdsandiford@googlemail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gerald@pfeifer.com \
    --cc=james.greenhalgh@arm.com \
    --cc=joseph@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).