Re: [PATCH] Update mtfsf and mtfsfi instructions to support new optional operands.

[Alan Modra <amodra at bigpond dot net dot au>]

On Tue, May 15, 2007 at 04:34:41PM -0500, Peter Bergner wrote:
> The following patch adds support for new optional operands (as of POWER6)
> on the mtfsf and mtfsfi instructions.  The mtfsf change was a little more
> difficult, since we added two extra optional operands, so the accepted
> form has either 2 operands or 4, but not three.  This lead me to invent
> the PPC_OPERAND_OPTIONAL_GROUP hackery used below, so we either omit both
> optional operands or print them both.

I think we ought to be doing that for all instructions having more
than one optional operand (tlbre, tlbwe).  The disassembler should
behave similarly to the assembler, ie. on seeing the first optional
operand, scan over all remaining operands to see whether all optional
operands can be omitted.  Then print all of them, or skip all of
them.  So I don't think you need the new PPC_OPERAND_OPTIONAL_GROUP
flag.  (If we really do need PPC_OPERAND_OPTIONAL_GROUP some time in
the future, then we also need an assembler change..)

> +static int
> +operand_is_optional_powerpc (const struct powerpc_operand *operand,
[snip]
> +      *operand_cnt += 1;

I think you're setting this skip count too soon (eg. given 3 optional
operands, with the first two values zero but the third non-zero).

> +      int is_optional;
> +      int operand_cnt;

In line with the above, I think we want one boolean saying that we
have already tested optional operands (so don't do it again), and
another saying to skip all optional operands.  Or use one 3-value
flag.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre