Hi!
On Wed, Sep 01, 2021 at 11:13:47AM -0500, Bill Schmidt wrote:
> Provide replacements for htm_spr_num and htm_expand_builtin. No logic
> changes are intended here, as usual. Much code was factored out into
> rs6000_expand_new_builtin, so the new version of htm_expand_builtin is
> a little tidier.
Nice.
> Also implement the support for the "endian" and "32bit" attributes,
> which is straightforward. These just do icode substitution.
Don't call this "attributes" please? I don't know what would be a
better name, of course. "bif attribute" maybe?
> + rtx op[MAX_HTM_OPERANDS], pat;
Don't declare arrays and scalars in the same statement, in general. It
is important that the arrays stand out.
Also, don't declare things before they are used please.
> + FOR_EACH_CALL_EXPR_ARG (arg, iter, exp)
> + {
> + if (arg == error_mark_node || nopnds >= MAX_HTM_OPERANDS)
> + return const0_rtx;
> +
> + insn_op = &insn_data[icode].operand[nopnds];
> + op[nopnds] = expand_normal (arg);
> +
> + if (!insn_op->predicate (op[nopnds], insn_op->mode))
> + {
> + if (!strcmp (insn_op->constraint, "n"))
> + {
> + int arg_num = (nonvoid) ? nopnds : nopnds + 1;
Please don't parenthesise random expressions like "nonvoid". I wonder
if that can be simpler handled by just unshifting a void_node into the
operands, btw :-)
And the same "n" thing as before of course. Since it is the same: some
factoring would be helpful probably.
> + machine_mode mode = (TARGET_POWERPC64) ? DImode : SImode;
Superfluous parens. This is just "word_mode", anyway?
> + /* If this builtin accesses a CR, then pass in a scratch
> + CR as the last operand. */
> + else if (bif_is_htmcr (*bifaddr))
> + {
> + cr = gen_reg_rtx (CCmode);
> + op[nopnds++] = cr;
> + }
There is only one CR ("condition register"). You can say CRF here
("condition register field", a 4-bit thing), or just cc or CC maybe
("condition code"). A pet peeve, I know.
> + if (bif_is_endian (*bifaddr) && BYTES_BIG_ENDIAN)
"is_endian" should maybe be "is_bigendian" or something like that?
Okay for trunk with the changes you see fit at this time. Thanks!
Segher
