Re: Repost [PATCH 3/6] PowerPC: Add support for accumulators in DMR registers.

[Michael Meissner <meissner@linux.ibm.com>]

On Wed, Feb 07, 2024 at 05:38:46PM +0800, Kewen.Lin wrote:
> >>> -(define_insn_and_split "*movxo"
> >>> +(define_insn_and_split "*movxo_nodm"
> >>>    [(set (match_operand:XO 0 "nonimmediate_operand" "=d,ZwO,d")
> >>>  	(match_operand:XO 1 "input_operand" "ZwO,d,d"))]
> >>> -  "TARGET_MMA
> >>> +  "TARGET_MMA && !TARGET_DENSE_MATH
> >>>     && (gpc_reg_operand (operands[0], XOmode)
> >>>         || gpc_reg_operand (operands[1], XOmode))"
> >>>    "@
> >>> @@ -366,6 +369,31 @@ (define_insn_and_split "*movxo"
> >>>     (set_attr "length" "*,*,16")
> >>>     (set_attr "max_prefixed_insns" "2,2,*")])
> >>>  
> >>> +(define_insn_and_split "*movxo_dm"
> >>> +  [(set (match_operand:XO 0 "nonimmediate_operand" "=wa,QwO,wa,wD,wD,wa")
> >>> +	(match_operand:XO 1 "input_operand"        "QwO,wa, wa,wa,wD,wD"))]
> >>
> >> Why not adopt ZwO rather than QwO?
> > 
> > You have to split the address into 2 addresses for loading or storing vector
> > pairs (or 4 addresses for loading or storing vectors).  Z would allow
> > register+register addresses, and you wouldn't be able to create the second 
> > address by adding 128 to it.  Hence it uses 'Q' for register only and 'wo' for
> > d-form addresses.
> 
> Thanks for clarifying.  But without this patch the define_insn_and_split *movxo
> adopts "ZwO", IMHO it would mean the current "*movxo" define_insn_and_split have
> been problematic?  I thought adjust_address can ensure the new address would be
> still valid after adjusting 128 offset, could you double check?

Well it is more of a theoretical bug.  Using 'Z' is wrong as I said because after
register allocation you can't split an x-form (register+register) address.
Using 'Q' would not allow reg+reg but would allow reg, which can be split
because the 2nd address will be a d-form (reg+offset).

But in practice, it won't be an issue since rs6000_setup_reg_addr_masks won't
allow reg+reg addresses for TDOmode.

> >>>  #ifdef TARGET_REGNAMES
> >>> @@ -1250,6 +1255,8 @@ static const char alt_reg_names[][8] =
> >>>    "%cr0",  "%cr1", "%cr2", "%cr3", "%cr4", "%cr5", "%cr6", "%cr7",
> >>>    /* vrsave vscr sfp */
> >>>    "vrsave", "vscr", "sfp",
> >>> +  /* DMRs */
> >>> +  "%dmr0", "%dmr1", "%dmr2", "%dmr3", "%dmr4", "%dmr5", "%dmr6", "%dmr7",
> >>
> >> Should be without "r" here, as tested gas doesn't recognize %dmr0 but it does
> >> recognize %dm0.
> 
> I guessed some reply was missing on this part (and some latter others)?  Just want
> to ensure something wasn't missing and hope this helps.  :)

I missed this on the first round of comments, but if gas doesn't like %dmr2 we
should use %dm2.

Thanks for catching this.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com