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

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

On Thu, Jan 25, 2024 at 05:28:49PM +0800, Kewen.Lin wrote:
> 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.

Ok.

> > 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.

That depends on whether there will be other things added in the future power
that are not in the MMA+ instruction set.

But I can switch to defining TARGET_DENSE_MATH to testing TARGET_FUTURE and
TARGET_MMA.  That way if/when a new cpu comes out, we will just have to change
the definition of TARGET_DENSE_MATH and not all of the uses.

I will also add TARGET_MMA_NO_DENSE_MATH to handle the existing MMA code for
assemble and disassemble when we don't have dense math instructions.

> > 
> > 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".

Ok.

> > 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.

Ok.

> >    ])
> >  
> >  (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//

Ok.

> > +;; 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?

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.

> 
> > +  "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?

Ok.

> 
> >     (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?

Ok.

> >    [(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.

Ok.

> > + "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/?

Ok

> 
> > +		       (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.

Ok.
> 
> >    "#"
> >    "&& 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
> 

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