public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Kewen.Lin" <linkw@linux.ibm.com>
To: Michael Meissner <meissner@linux.ibm.com>
Cc: gcc-patches@gcc.gnu.org,
	Segher Boessenkool <segher@kernel.crashing.org>,
	David Edelsohn <dje.gcc@gmail.com>,
	Peter Bergner <bergner@linux.ibm.com>
Subject: Re: Repost [PATCH 3/6] PowerPC: Add support for accumulators in DMR registers.
Date: Thu, 25 Jan 2024 17:28:49 +0800	[thread overview]
Message-ID: <d40688c4-42db-7df6-3b64-84ce28a992de@linux.ibm.com> (raw)
In-Reply-To: <ZZiS7-05Y1n48bjk@cowardly-lion.the-meissners.org>

Hi Mike,

on 2024/1/6 07:38, Michael Meissner wrote:
> The MMA subsystem added the notion of accumulator registers as an optional
> feature of ISA 3.1 (power10).  In ISA 3.1, these accumulators overlapped with
> the traditional floating point registers 0..31, but logically the accumulator
> registers were separate from the FPR registers.  In ISA 3.1, it was anticipated

Using VSX register 0..31 rather than traditional floating point registers 0..31
seems more clear, since floating point registers imply 64 bit long registers.

> that in future systems, the accumulator registers may no overlap with the FPR
> registers.  This patch adds the support for dense math registers as separate
> registers.
> 
> This particular patch does not change the MMA support to use the accumulators
> within the dense math registers.  This patch just adds the basic support for
> having separate DMRs.  The next patch will switch the MMA support to use the
> accumulators if -mcpu=future is used.
> 
> For testing purposes, I added an undocumented option '-mdense-math' to enable
> or disable the dense math support.

Can we avoid this and use one macro for it instead?  As you might have noticed
that some previous temporary options like -mpower{8,9}-vector cause ICEs due to
some unexpected combination and we are going to neuter them, so let's try our
best to avoid it if possible.  I guess one macro TARGET_DENSE_MATH defined by
TARGET_FUTURE && TARGET_MMA matches all use places? and specifying -mcpu=future
can enable it while -mcpu=power10 can disable it.

> 
> This patch adds a new constraint (wD).  If MMA is selected but dense math is
> not selected (i.e. -mcpu=power10), the wD constraint will allow access to
> accumulators that overlap with the VSX vector registers 0..31.  If both MMA and

Sorry for nitpicking, it's more accurate with "VSX registers 0..31".

> dense math are selected (i.e. -mcpu=future), the wD constraint will only allow
> dense math registers.
> 
> This patch modifies the existing %A output modifier.  If MMA is selected but
> dense math is not selected, then %A output modifier converts the VSX register
> number to the accumulator number, by dividing it by 4.  If both MMA and dense
> math are selected, then %A will map the separate DMR registers into 0..7.
> 
> The intention is that user code using extended asm can be modified to run on
> both MMA without dense math and MMA with dense math:
> 
>     1)	If possible, don't use extended asm, but instead use the MMA built-in
> 	functions;
> 
>     2)	If you do need to write extended asm, change the d constraints
> 	targetting accumulators should now use wD;
> 
>     3)	Only use the built-in zero, assemble and disassemble functions create
> 	move data between vector quad types and dense math accumulators.
> 	I.e. do not use the xxmfacc, xxmtacc, and xxsetaccz directly in the
> 	extended asm code.  The reason is these instructions assume there is a
> 	1-to-1 correspondence between 4 adjacent FPR registers and an
> 	accumulator that overlaps with those instructions.  With accumulators
> 	now being separate registers, there no longer is a 1-to-1
> 	correspondence.
> 
> It is possible that the mangling for DMRs and the GDB register numbers may
> change in the future.
> 
> 2024-01-05   Michael Meissner  <meissner@linux.ibm.com>
> 
> gcc/
> 
> 	* config/rs6000/constraints.md (wD constraint): New constraint.
> 	* config/rs6000/mma.md (UNSPEC_DM_ASSEMBLE_ACC): New unspec.
> 	(movxo): Convert into define_expand.
> 	(movxo_vsx): Version of movxo where accumulators overlap with VSX vector
> 	registers 0..31.
> 	(movxo_dm): Verson of movxo that supports separate dense math
> 	accumulators.
> 	(mma_assemble_acc): Add dense math support to define_expand.
> 	(mma_assemble_acc_vsx): Rename from mma_assemble_acc, and restrict it to
> 	non dense math systems.
> 	(mma_assemble_acc_dm): Dense math version of mma_assemble_acc.
> 	(mma_disassemble_acc): Add dense math support to define_expand.
> 	(mma_disassemble_acc_vsx): Rename from mma_disassemble_acc, and restrict
> 	it to non dense math systems.
> 	(mma_disassemble_acc_dm): Dense math version of mma_disassemble_acc.
> 	* config/rs6000/predicates.md (dmr_operand): New predicate.
> 	(accumulator_operand): Likewise.
> 	* config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS): Add -mdense-math.
> 	(POWERPC_MASKS): Likewise.
> 	* config/rs6000/rs6000.cc (enum rs6000_reg_type): Add DMR_REG_TYPE.
> 	(enum rs6000_reload_reg_type): Add RELOAD_REG_DMR.
> 	(LAST_RELOAD_REG_CLASS): Add support for DMR registers and the wD
> 	constraint.
> 	(reload_reg_map): Likewise.
> 	(rs6000_reg_names): Likewise.
> 	(alt_reg_names): Likewise.
> 	(rs6000_hard_regno_nregs_internal): Likewise.
> 	(rs6000_hard_regno_mode_ok_uncached): Likewise.
> 	(rs6000_debug_reg_global): Likewise.
> 	(rs6000_setup_reg_addr_masks): Likewise.
> 	(rs6000_init_hard_regno_mode_ok): Likewise.
> 	(rs6000_option_override_internal): Add checking for -mdense-math.
> 	(rs6000_secondary_reload_memory): Add support for DMR registers.
> 	(rs6000_secondary_reload_simple_move): Likewise.
> 	(rs6000_preferred_reload_class): Likewise.
> 	(rs6000_secondary_reload_class): Likewise.
> 	(print_operand): Make %A handle both FPRs and DMRs.
> 	(rs6000_dmr_register_move_cost): New helper function.
> 	(rs6000_register_move_cost): Add support for DMR registers.
> 	(rs6000_memory_move_cost): Likewise.
> 	(rs6000_compute_pressure_classes): Likewise.
> 	(rs6000_debugger_regno): Likewise.
> 	(rs6000_opt_masks): Add -mdense-math.
> 	(rs6000_split_multireg_move): Add support for DMRs.
> 	* config/rs6000/rs6000.h (UNITS_PER_DMR_WORD): New macro.
> 	(FIRST_PSEUDO_REGISTER): Update for DMRs.
> 	(FIXED_REGISTERS): Add DMRs.
> 	(CALL_REALLY_USED_REGISTERS): Likewise.
> 	(REG_ALLOC_ORDER): Likewise.
> 	(enum reg_class): Add DM_REGS.
> 	(REG_CLASS_NAMES): Likewise.
> 	(REG_CLASS_CONTENTS): Likewise.
> 	* config/rs6000/rs6000.md (FIRST_DMR_REGNO): New constant.
> 	(LAST_DMR_REGNO): Likewise.
> 	(isa attribute): Add 'dm' and 'not_dm' attributes.
> 	(enabled attribute): Support 'dm' and 'not_dm' attributes.
> 	* config/rs6000/rs6000.opt (-mdense-math): New switch.
> 	* doc/md.texi (PowerPC constraints): Document wD constraint.
> ---
>  gcc/config/rs6000/constraints.md  |   3 +
>  gcc/config/rs6000/mma.md          | 115 ++++++++++++------
>  gcc/config/rs6000/predicates.md   |  32 +++++
>  gcc/config/rs6000/rs6000-cpus.def |   2 +
>  gcc/config/rs6000/rs6000.cc       | 189 ++++++++++++++++++++++++++----
>  gcc/config/rs6000/rs6000.h        |  38 +++++-
>  gcc/config/rs6000/rs6000.md       |  12 +-
>  gcc/config/rs6000/rs6000.opt      |   4 +
>  gcc/doc/md.texi                   |   7 ++
>  9 files changed, 343 insertions(+), 59 deletions(-)
> 
> diff --git a/gcc/config/rs6000/constraints.md b/gcc/config/rs6000/constraints.md
> index c99997bf82b..614e431c085 100644
> --- a/gcc/config/rs6000/constraints.md
> +++ b/gcc/config/rs6000/constraints.md
> @@ -107,6 +107,9 @@ (define_constraint "wB"
>         (match_test "TARGET_P8_VECTOR")
>         (match_operand 0 "s5bit_cint_operand")))
>  
> +(define_register_constraint "wD" "rs6000_constraints[RS6000_CONSTRAINT_wD]"
> +  "Accumulator register.")
> +
>  (define_constraint "wE"
>    "@internal Vector constant that can be loaded with the XXSPLTIB instruction."
>    (match_test "xxspltib_constant_nosplit (op, mode)"))
> diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
> index 6a7d8a836db..bb898919ab5 100644
> --- a/gcc/config/rs6000/mma.md
> +++ b/gcc/config/rs6000/mma.md
> @@ -91,6 +91,7 @@ (define_c_enum "unspec"
>     UNSPEC_MMA_XVI8GER4SPP
>     UNSPEC_MMA_XXMFACC
>     UNSPEC_MMA_XXMTACC
> +   UNSPEC_DM_ASSEMBLE_ACC

The other UNSPEC.*ASSEMBLE like UNSPECV_MMA_ASSEMBLE don't have _ACC suffix,
it's better to keep consistent if this suffix doesn't distinguish something.

>    ])
>  
>  (define_c_enum "unspecv"
> @@ -321,7 +322,9 @@ (define_insn_and_split "*movoo"
>     (set_attr "length" "*,8,*,8,8")
>     (set_attr "isa" "lxvp,*,stxvp,*,*")])
>  \f
> -;; Vector quad support.  XOmode can only live in FPRs.
> +;; Vector quad support.  Under the original MMA, XOmode can only live in VSX
> +;; vector registers 0..31.  With dense math, XOmode can live in either VSX

Nit: s/vector//

> +;; registers (0..63) or DMR registers.
>  (define_expand "movxo"
>    [(set (match_operand:XO 0 "nonimmediate_operand")
>  	(match_operand:XO 1 "input_operand"))]
> @@ -346,10 +349,10 @@ (define_expand "movxo"
>      gcc_assert (false);
>  })
>  
> -(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?

> +  "TARGET_DENSE_MATH
> +   && (gpc_reg_operand (operands[0], XOmode)
> +       || gpc_reg_operand (operands[1], XOmode))"
> +  "@
> +   #
> +   #
> +   #
> +   dmxxinstdmr512 %0,%1,%Y1,0
> +   dmmr %0,%1
> +   dmxxextfdmr512 %0,%Y0,%1,0"
> +  "&& reload_completed
> +   && !dmr_operand (operands[0], XOmode)
> +   && !dmr_operand (operands[1], XOmode)"
> +  [(const_int 0)]
> +{
> +  rs6000_split_multireg_move (operands[0], operands[1]);
> +  DONE;
> +}
> +  [(set_attr "type" "vecload,vecstore,veclogical,mma,mma,mma")
> +   (set_attr "length" "*,*,16,*,*,*")
> +   (set_attr "max_prefixed_insns" "2,2,*,*,*,*")])
> +
>  (define_expand "vsx_assemble_pair"
>    [(match_operand:OO 0 "vsx_register_operand")
>     (match_operand:V16QI 1 "mma_assemble_input_operand")
> @@ -433,25 +461,38 @@ (define_insn_and_split "*vsx_disassemble_pair"
>  })
>  
>  (define_expand "mma_assemble_acc"
> -  [(match_operand:XO 0 "fpr_reg_operand")
> +  [(match_operand:XO 0 "register_operand")

Maybe use the newly introduced accumulator_operand?

>     (match_operand:V16QI 1 "mma_assemble_input_operand")
>     (match_operand:V16QI 2 "mma_assemble_input_operand")
>     (match_operand:V16QI 3 "mma_assemble_input_operand")
>     (match_operand:V16QI 4 "mma_assemble_input_operand")]
>    "TARGET_MMA"
>  {
> -  rtx src = gen_rtx_UNSPEC_VOLATILE (XOmode,
> -			    	     gen_rtvec (4, operands[1], operands[2],
> -				       		operands[3], operands[4]),
> -			    	     UNSPECV_MMA_ASSEMBLE);
> -  emit_move_insn (operands[0], src);
> +  rtx op0 = operands[0];
> +  rtx op1 = operands[1];
> +  rtx op2 = operands[2];
> +  rtx op3 = operands[3];
> +  rtx op4 = operands[4];
> +
> +  if (TARGET_DENSE_MATH)
> +    {
> +      rtx vpair1 = gen_reg_rtx (OOmode);
> +      rtx vpair2 = gen_reg_rtx (OOmode);
> +      emit_insn (gen_vsx_assemble_pair (vpair1, op1, op2));
> +      emit_insn (gen_vsx_assemble_pair (vpair2, op3, op4));
> +      emit_insn (gen_mma_assemble_acc_dm (op0, vpair1, vpair2));
> +    }
> +
> +  else
> +    emit_insn (gen_mma_assemble_acc_vsx (op0, op1, op2, op3, op4));
> +
>    DONE;
>  })
>  
>  ;; We cannot update the four output registers atomically, so mark the output
> -;; as an early clobber so we don't accidentally clobber the input operands.  */
> +;; as an early clobber so we don't accidentally clobber the input operands.
>  
> -(define_insn_and_split "*mma_assemble_acc"
> +(define_insn_and_split "mma_assemble_acc_vsx"

Nit: since we use "*_nodm" above, it seems better to name it with
"mma_assemble_acc_nodm" which has the same style?

>    [(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
>  	(unspec_volatile:XO
>  	  [(match_operand:V16QI 1 "mma_assemble_input_operand" "mwa")
> @@ -459,7 +500,7 @@ (define_insn_and_split "*mma_assemble_acc"
>  	   (match_operand:V16QI 3 "mma_assemble_input_operand" "mwa")
>  	   (match_operand:V16QI 4 "mma_assemble_input_operand" "mwa")]
>  	  UNSPECV_MMA_ASSEMBLE))]
> -  "TARGET_MMA
> +  "TARGET_MMA && !TARGET_DENSE_MATH
>     && fpr_reg_operand (operands[0], XOmode)"
>    "#"
>    "&& reload_completed"
> @@ -473,28 +514,31 @@ (define_insn_and_split "*mma_assemble_acc"
>    DONE;
>  })
>  
> +;; On a system with dense math, we build the accumulators from two vector
> +;; pairs.
> +
> +(define_insn "mma_assemble_acc_dm"
> + [(set (match_operand:XO 0 "dmr_operand" "=wD")
> +       (unspec:XO [(match_operand:OO 1 "vsx_register_operand" "wa")
> +		   (match_operand:OO 2 "vsx_register_operand" "wa")]
> +		  UNSPEC_DM_ASSEMBLE_ACC))]
> + "TARGET_MMA && TARGET_DENSE_MATH"

Nit: redundant TARGET_MMA checking.

> + "dmxxinstdmr512 %0,%1,%2,0"
> + [(set_attr "type" "mma")])
> +
>  (define_expand "mma_disassemble_acc"
> -  [(match_operand:V16QI 0 "mma_disassemble_output_operand")
> -   (match_operand:XO 1 "fpr_reg_operand")
> -   (match_operand 2 "const_0_to_3_operand")]
> -  "TARGET_MMA"
> -{
> -  rtx src;
> -  int regoff = INTVAL (operands[2]);
> -  src = gen_rtx_UNSPEC (V16QImode,
> -			gen_rtvec (2, operands[1], GEN_INT (regoff)),
> -			UNSPEC_MMA_EXTRACT);
> -  emit_move_insn (operands[0], src);
> -  DONE;
> -})
> +  [(set (match_operand:V16QI 0 "register_operand")
> +	(unspec:V16QI [(match_operand:XO 1 "register_operand")

s/register_operand/accumulator_operand/?

> +		       (match_operand 2 "const_0_to_3_operand")]
> +		      UNSPEC_MMA_EXTRACT))]
> +  "TARGET_MMA")
>  
> -(define_insn_and_split "*mma_disassemble_acc"
> +(define_insn_and_split "*mma_disassemble_acc_vsx"
>    [(set (match_operand:V16QI 0 "mma_disassemble_output_operand" "=mwa")
> -       (unspec:V16QI [(match_operand:XO 1 "fpr_reg_operand" "d")
> -		      (match_operand 2 "const_0_to_3_operand")]
> +	(unspec:V16QI [(match_operand:XO 1 "fpr_reg_operand" "d")
> +		       (match_operand 2 "const_0_to_3_operand")]
>  		      UNSPEC_MMA_EXTRACT))]
> -  "TARGET_MMA
> -   && fpr_reg_operand (operands[1], XOmode)"
> +  "TARGET_MMA"

Do we still expect to see this pattern if TARGET_DENSE_MATH?
If no, we should guard the condition with !TARGET_DENSE_MATH.

>    "#"
>    "&& reload_completed"
>    [(const_int 0)]
> @@ -506,9 +550,14 @@ (define_insn_and_split "*mma_disassemble_acc"
>    DONE;
>  })
>  
> -;; MMA instructions that do not use their accumulators as an input, still
> -;; must not allow their vector operands to overlap the registers used by
> -;; the accumulator.  We enforce this by marking the output as early clobber.
> +(define_insn "*mma_disassemble_acc_dm"
> +  [(set (match_operand:V16QI 0 "vsx_register_operand" "=wa")
> +	(unspec:V16QI [(match_operand:XO 1 "dmr_operand" "wD")
> +		       (match_operand 2 "const_0_to_3_operand")]
> +		      UNSPEC_MMA_EXTRACT))]
> +  "TARGET_DENSE_MATH"
> +  "dmxxextfdmr256 %0,%1,2"
> +  [(set_attr "type" "mma")])
>  
>  (define_insn "mma_<acc>"
>    [(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
> diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
> index d23ce9a77a3..3040dcd50a3 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -186,6 +186,38 @@ (define_predicate "vlogical_operand"
>    return VLOGICAL_REGNO_P (REGNO (op));
>  })
>  
> +;; Return 1 if op is a DMR register
> +(define_predicate "dmr_operand"
> +  (match_operand 0 "register_operand")
> +{
> +  if (!REG_P (op))
> +    return 0;
> +
> +  if (!HARD_REGISTER_P (op))
> +    return 1;
> +
> +  return DMR_REGNO_P (REGNO (op));
> +})
> +
> +;; Return 1 if op is an accumulator.  On power10 systems, the accumulators
> +;; overlap with the FPRs, while on systems with dense math, the accumulators
> +;; are separate dense math registers and do not overlap with the FPR
> +;; registers..

Nit: an unexpected "."?

> +(define_predicate "accumulator_operand"
> +  (match_operand 0 "register_operand")
> +{

fpr_reg_operand checks for subreg as well, should we check for it here as well?

> +  if (!REG_P (op))
> +    return 0;
> +
> +  if (!HARD_REGISTER_P (op))
> +    return 1;
> +
> +  int r = REGNO (op);
> +  return (TARGET_DENSE_MATH
> +	  ? DMR_REGNO_P (r)
> +	  : FP_REGNO_P (r) && (r & 3) == 0);
> +})
> +
>  ;; Return 1 if op is the carry register.
>  (define_predicate "ca_operand"
>    (match_operand 0 "register_operand")
> diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def
> index b6cd6d8cc84..4621b97b522 100644
> --- a/gcc/config/rs6000/rs6000-cpus.def
> +++ b/gcc/config/rs6000/rs6000-cpus.def
> @@ -91,6 +91,7 @@
>  /* Flags for a potential future processor that may or may not be delivered.  */
>  #define ISA_FUTURE_MASKS	(ISA_3_1_MASKS_SERVER			\
>  				 | OPTION_MASK_BLOCK_OPS_VECTOR_PAIR	\
> +				 | OPTION_MASK_DENSE_MATH		\
>  				 | OPTION_MASK_FUTURE)
>  
>  /* Flags that need to be turned off if -mno-power9-vector.  */
> @@ -134,6 +135,7 @@
>  				 | OPTION_MASK_DFP			\
>  				 | OPTION_MASK_DIRECT_MOVE		\
>  				 | OPTION_MASK_DLMZB			\
> +				 | OPTION_MASK_DENSE_MATH		\
>  				 | OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
>  				 | OPTION_MASK_FLOAT128_HW		\
>  				 | OPTION_MASK_FLOAT128_KEYWORD		\
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index bc509399cf6..83e32f7a43a 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -290,7 +290,8 @@ enum rs6000_reg_type {
>    ALTIVEC_REG_TYPE,
>    FPR_REG_TYPE,
>    SPR_REG_TYPE,
> -  CR_REG_TYPE
> +  CR_REG_TYPE,
> +  DMR_REG_TYPE
>  };
>  
>  /* Map register class to register type.  */
> @@ -304,22 +305,23 @@ static enum rs6000_reg_type reg_class_to_reg_type[N_REG_CLASSES];
>  
>  
>  /* Register classes we care about in secondary reload or go if legitimate
> -   address.  We only need to worry about GPR, FPR, and Altivec registers here,
> -   along an ANY field that is the OR of the 3 register classes.  */
> +   address.  We only need to worry about GPR, FPR, Altivec, and DMR registers
> +   here, along an ANY field that is the OR of the 4 register classes.  */
>  
>  enum rs6000_reload_reg_type {
>    RELOAD_REG_GPR,			/* General purpose registers.  */
>    RELOAD_REG_FPR,			/* Traditional floating point regs.  */
>    RELOAD_REG_VMX,			/* Altivec (VMX) registers.  */
> -  RELOAD_REG_ANY,			/* OR of GPR, FPR, Altivec masks.  */
> +  RELOAD_REG_DMR,			/* DMR registers.  */
> +  RELOAD_REG_ANY,			/* OR of GPR/FPR/VMX/DMR masks.  */
>    N_RELOAD_REG
>  };
>  
> -/* For setting up register classes, loop through the 3 register classes mapping
> +/* For setting up register classes, loop through the 4 register classes mapping
>     into real registers, and skip the ANY class, which is just an OR of the
>     bits.  */
>  #define FIRST_RELOAD_REG_CLASS	RELOAD_REG_GPR
> -#define LAST_RELOAD_REG_CLASS	RELOAD_REG_VMX
> +#define LAST_RELOAD_REG_CLASS	RELOAD_REG_DMR
>  
>  /* Map reload register type to a register in the register class.  */
>  struct reload_reg_map_type {
> @@ -331,6 +333,7 @@ static const struct reload_reg_map_type reload_reg_map[N_RELOAD_REG] = {
>    { "Gpr",	FIRST_GPR_REGNO },	/* RELOAD_REG_GPR.  */
>    { "Fpr",	FIRST_FPR_REGNO },	/* RELOAD_REG_FPR.  */
>    { "VMX",	FIRST_ALTIVEC_REGNO },	/* RELOAD_REG_VMX.  */
> +  { "DMR",	FIRST_DMR_REGNO },	/* RELOAD_REG_DMR.  */
>    { "Any",	-1 },			/* RELOAD_REG_ANY.  */
>  };
>  
> @@ -1224,6 +1227,8 @@ char rs6000_reg_names[][8] =
>        "0",  "1",  "2",  "3",  "4",  "5",  "6",  "7",
>    /* vrsave vscr sfp */
>        "vrsave", "vscr", "sfp",
> +  /* DMRs */
> +      "0", "1", "2", "3", "4", "5", "6", "7",
>  };
>  
>  #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.

>  };
>  #endif
>  
> @@ -1846,6 +1853,9 @@ rs6000_hard_regno_nregs_internal (int regno, machine_mode mode)
>    else if (ALTIVEC_REGNO_P (regno))
>      reg_size = UNITS_PER_ALTIVEC_WORD;
>  
> +  else if (DMR_REGNO_P (regno))
> +    reg_size = UNITS_PER_DMR_WORD;
> +
>    else
>      reg_size = UNITS_PER_WORD;
>  
> @@ -1867,9 +1877,36 @@ rs6000_hard_regno_mode_ok_uncached (int regno, machine_mode mode)
>    if (mode == OOmode)
>      return (TARGET_MMA && VSX_REGNO_P (regno) && (regno & 1) == 0);
>  
> -  /* MMA accumulator modes need FPR registers divisible by 4.  */
> +  /* On ISA 3.1 (power10), MMA accumulator modes need FPR registers divisible
> +     by 4.
> +
> +     If dense math is enabled, allow all VSX registers plus the DMR registers.
> +     We need to make sure we don't cross between the boundary of FPRs and
> +     traditional Altiviec registers.  */
>    if (mode == XOmode)
> -    return (TARGET_MMA && FP_REGNO_P (regno) && (regno & 3) == 0);
> +    {
> +      if (TARGET_MMA && !TARGET_DENSE_MATH)
> +	return (FP_REGNO_P (regno) && (regno & 3) == 0);
> +
> +      else if (TARGET_DENSE_MATH)
> +	{
> +	  if (DMR_REGNO_P (regno))
> +	    return 1;
> +
> +	  if (FP_REGNO_P (regno))
> +	    return ((regno & 1) == 0 && regno <= LAST_FPR_REGNO - 3);
> +
> +	  if (ALTIVEC_REGNO_P (regno))
> +	    return ((regno & 1) == 0 && regno <= LAST_ALTIVEC_REGNO - 3);
> +	}

I could miss something, I didn't find which section of RFC indicates this
restriction, could you please point out for me?  Thanks!

> +
> +      else
> +	return 0;
> +    }
> +
> +  /* No other types other than XOmode can go in DMRs.  */
> +  if (DMR_REGNO_P (regno))
> +    return 0;
>  
>    /* PTImode can only go in GPRs.  Quad word memory operations require even/odd
>       register combinations, and use PTImode where we need to deal with quad
> @@ -2312,6 +2349,7 @@ rs6000_debug_reg_global (void)
>    rs6000_debug_reg_print (FIRST_ALTIVEC_REGNO,
>  			  LAST_ALTIVEC_REGNO,
>  			  "vs");
> +  rs6000_debug_reg_print (FIRST_DMR_REGNO, LAST_DMR_REGNO, "dmr");

Nit: Like above, use 'dm'.

>    rs6000_debug_reg_print (LR_REGNO, LR_REGNO, "lr");
>    rs6000_debug_reg_print (CTR_REGNO, CTR_REGNO, "ctr");
>    rs6000_debug_reg_print (CR0_REGNO, CR7_REGNO, "cr");
> @@ -2332,6 +2370,7 @@ rs6000_debug_reg_global (void)
>  	   "wr reg_class = %s\n"
>  	   "wx reg_class = %s\n"
>  	   "wA reg_class = %s\n"
> +	   "wD reg_class = %s\n"
>  	   "\n",
>  	   reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_d]],
>  	   reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_v]],
> @@ -2339,7 +2378,8 @@ rs6000_debug_reg_global (void)
>  	   reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_we]],
>  	   reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wr]],
>  	   reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wx]],
> -	   reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wA]]);
> +	   reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wA]],
> +	   reg_class_names[rs6000_constraints[RS6000_CONSTRAINT_wD]]);
> 

snip ...

> +/* Subroutine to determine the move cost of dense math registers.  If we are
> +   moving to/from VSX_REGISTER registers, the cost is either 1 move (for
> +   512-bit accumulators) or 2 moves (for 1,024 dmr registers).  If we are
> +   moving to anything else like GPR registers, make the cost very high.  */
> +
> +static int
> +rs6000_dmr_register_move_cost (machine_mode mode, reg_class_t rclass)
> +{
> +  const int reg_move_base = 2;
> +  HARD_REG_SET vsx_set = (reg_class_contents[rclass]
> +			  & reg_class_contents[VSX_REGS]);
> +
> +  if (TARGET_DENSE_MATH && !hard_reg_set_empty_p (vsx_set))

Can we just use reg_classes_intersect_p (rclass, VSX_REGS)?

> +    {
> +      /* __vector_quad (i.e. XOmode) is tranfered in 1 instruction.  */
> +      if (mode == XOmode)
> +	return reg_move_base;
> +
> +      else
> +	return reg_move_base * 2 * hard_regno_nregs (FIRST_DMR_REGNO, mode);

I guess this "else" arm is for TDOmode, which belongs to that patch.

> +    }
> +
> +  return 1000 * 2 * hard_regno_nregs (FIRST_DMR_REGNO, mode);
> +}
> +
>  /* A C expression returning the cost of moving data from a register of class
>     CLASS1 to one of CLASS2.  */
>  
> @@ -22843,17 +22969,28 @@ rs6000_register_move_cost (machine_mode mode,
>    if (TARGET_DEBUG_COST)
>      dbg_cost_ctrl++;
>  

snip ...

>  /* Table of additional register names to use in user input.  */
> @@ -2132,6 +2158,8 @@ extern char rs6000_reg_names[][8];	/* register names (0 vs. %r0).  */
>    {"vs52", 84}, {"vs53", 85}, {"vs54", 86}, {"vs55", 87},	\
>    {"vs56", 88}, {"vs57", 89}, {"vs58", 90}, {"vs59", 91},	\
>    {"vs60", 92}, {"vs61", 93}, {"vs62", 94}, {"vs63", 95},	\
> +  {"dmr0", 111}, {"dmr1", 112}, {"dmr2", 113}, {"dmr3", 114},	\
> +  {"dmr4", 115}, {"dmr5", 116}, {"dmr6", 117}, {"dmr7", 118},	\

Nit: maybe s/dmr/dm/ to align the previous regnames.

>  }
>  
>  /* This is how to output an element of a case-vector that is relative.  */
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index a125fd8fc99..72af3e6ef70 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -51,6 +51,8 @@ (define_constants
>     (VRSAVE_REGNO		108)
>     (VSCR_REGNO			109)
>     (FRAME_POINTER_REGNUM	110)
> +   (FIRST_DMR_REGNO		111)
> +   (LAST_DMR_REGNO		118)
>    ])
>  
>  ;;
> @@ -355,7 +357,7 @@ (define_attr "cpu"
>    (const (symbol_ref "(enum attr_cpu) rs6000_tune")))
>  
>  ;; The ISA we implement.
> -(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9,p9v,p9kf,p9tf,p10,lxvp,stxvp"
> +(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9,p9v,p9kf,p9tf,p10,lxvp,stxvp,dm,not_dm"

Nit: s/not_dm/nodm/ to align with some previous wording.

BR,
Kewen


  parent reply	other threads:[~2024-01-25  9:28 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-05 23:27 Repost [PATCH 0/6] PowerPC Future patches Michael Meissner
2024-01-05 23:35 ` Repost [PATCH 1/6] Add -mcpu=future Michael Meissner
2024-01-19 18:43   ` Ping " Michael Meissner
2024-01-23  8:44   ` Repost " Kewen.Lin
2024-02-06  6:01     ` Michael Meissner
2024-02-07  9:21       ` Kewen.Lin
2024-02-07 19:58         ` Michael Meissner
2024-02-20 10:35           ` Kewen.Lin
2024-02-21  7:19             ` Michael Meissner
2024-02-26 10:46               ` Kewen.Lin
2024-02-23 17:57             ` Segher Boessenkool
2024-02-08 18:42         ` Segher Boessenkool
2024-02-08 18:35       ` Segher Boessenkool
2024-02-08 20:10   ` Segher Boessenkool
2024-01-05 23:37 ` Repost [PATCH 2/6] PowerPC: Make -mcpu=future enable -mblock-ops-vector-pair Michael Meissner
2024-01-19 18:44   ` Ping " Michael Meissner
2024-01-23  8:54   ` Repost " Kewen.Lin
2024-01-05 23:38 ` Repost [PATCH 3/6] PowerPC: Add support for accumulators in DMR registers Michael Meissner
2024-01-19 18:46   ` Ping " Michael Meissner
2024-01-25  9:28   ` Kewen.Lin [this message]
2024-02-07  0:06     ` Repost " Michael Meissner
2024-02-07  9:38       ` Kewen.Lin
2024-02-08  0:26         ` Michael Meissner
2024-01-05 23:39 ` Repost [PATCH 4/6] PowerPC: Make MMA insns support " Michael Meissner
2024-01-19 18:47   ` Ping " Michael Meissner
2024-02-04  3:21   ` Repost " Kewen.Lin
2024-02-07  3:31     ` Michael Meissner
2024-01-05 23:40 ` Repost [PATCH 5/6] PowerPC: Switch to dense math names for all MMA operations Michael Meissner
2024-01-19 18:48   ` Ping " Michael Meissner
2024-02-04  5:47   ` Repost " Kewen.Lin
2024-02-07 20:01     ` Michael Meissner
2024-01-05 23:42 ` Repost [PATCH 6/6] PowerPC: Add support for 1,024 bit DMR registers Michael Meissner
2024-01-19 18:49   ` Ping " Michael Meissner
2024-02-05  3:58   ` Repost " Kewen.Lin
2024-02-08  0:35     ` Michael Meissner
2024-02-08 18:22 ` Repost [PATCH 0/6] PowerPC Future patches Segher Boessenkool

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=d40688c4-42db-7df6-3b64-84ce28a992de@linux.ibm.com \
    --to=linkw@linux.ibm.com \
    --cc=bergner@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=meissner@linux.ibm.com \
    --cc=segher@kernel.crashing.org \
    /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).