Hi Mike,
Some comments on this patch:
On Wed, Aug 14, 2019 at 05:59:13PM -0400, Michael Meissner wrote:
> Due to some of the existing load and store insns not using the traditional
> operands[0] and operands[1], the functions that test whether an insn is
> prefixed only use the insn and not the operands directly.
Both the "update" and the "indexed" attributes have no problem with
this: the insns that have the problem set the attribute value directly.
This is mainly all the various update insns, but there are a bunch more,
and they all need different settings for their attributes.
> * config/rs6000/rs6000.c (rs6000_emit_move): Add support for
> loading up pc-relatve addresses.
(typo btw)
> +void
> +rs6000_final_prescan_insn (rtx_insn *insn)
> +{
> + next_insn_prefixed_p = (get_attr_prefixed (insn) != PREFIXED_NO);
> + return;
> +}
Don't say "return;" at the end of a function please.
> +void
> +rs6000_asm_output_opcode (FILE *stream, const char *)
> +{
> + if (next_insn_prefixed_p)
> + {
> + next_insn_prefixed_p = false;
> + fprintf (stream, "p");
> + }
You don't need to clear the flag here; the next call to
rs6000_final_prescan_insn will.
> +#define FINAL_PRESCAN_INSN(INSN, OPERANDS, NOPERANDS)
> \
> +do \
> + { \
> + if (TARGET_PREFIXED_ADDR)
> \
> + rs6000_final_prescan_insn (INSN);
> \
> + } \
> +while (0)
Either have the function only do what it needs to for prefixed, and call
it something with prefixed in the name, or put the TARGET_PREFIXED_ADDR
test in the function itself.
> +;; Load up a pc-relative address. ASM_OUTPUT_OPCODE will emit the initial
> "p".
> +(define_insn "*pcrel_addr"
> + [(set (match_operand:DI 0 "gpc_reg_operand" "=b*r")
> + (match_operand:DI 1 "pcrel_address"))]
> + "TARGET_PCREL"
> + "la %0,%a1"
> + [(set_attr "prefixed" "yes")])
(use P for addresses please)
> +;; Load up a pc-relative address to an external symbol. If the symbol and
> the
> +;; program are both defined in the main program, the linker will optimize
> this
> +;; to a PADDI. Otherwise, it will create a GOT address that is relocated by
> +;; the dynamic linker and loaded up.
> +(define_insn "*pcrel_ext_addr"
> + [(set (match_operand:DI 0 "gpc_reg_operand" "=b*r")
> + (match_operand:DI 1 "pcrel_external_address"))]
> + "TARGET_PCREL"
> + "ld %0,%a1"
> + [(set_attr "prefixed" "yes")])
pld does an indirection more than pla does, but this is not clear at all
from the RTL, from the predicate names. All this is *before* the linker
has done its thing, so pcrel_external_address is really some GOT memory,
so it should have that in its name.
Segher
