PowerPC POWER10 updates to dcbf, sync and wait instructions

PowerPC POWER10 updates to dcbf, sync and wait instructions

[Peter Bergner]

The following patch adds some additional power10 instruction support
that adds additional operands to the dcbf, sync and wait instructions.
I have also extended insert_ls() to catch some illegal operand usage
we were not catching before.  I've verified this has not caused any
regressions in the testsuite, so I plan on pushing this on Monday
unless someone (Alan?) sees something untoward.

Peter


opcodes/
	* ppc-opc.c (insert_ls, extract_ls): Handle 3-bit L fields and new
	WC values on POWER10 sync, dcbf  and wait instructions.
	(insert_pl, extract_pl): New functions.
	(L2OPT, LS, WC): Use insert_ls and extract_ls.
	(LS3): New , 3-bit L for sync.
	(LS3, L3OPT): New, 3-bit L for sync and dcbf.
	(SC2, PL): New, 2-bit SC and PL for sync and wait.
	(XWCPL_MASK, XL3RT_MASK, XSYNCLS_MASK): New instruction masks.
	(XOPL3, XWCPL, XSYNCLS): New opcode macros.
	(powerpc_opcodes) <dcbflp, dcbfps, dcbstps pause_short, phwsync,
	plwsync, stcisync, stncisync, stsync, waitrsv>: New extended mnemonics.
	<wait>: Enable PL operand on POWER10.
	<dcbf>: Enable L3OPT operand on POWER10.
	<sync>: Enable SC2 operand on POWER10.

gas/
	* testsuite/gas/ppc/power9.s <dcbf, dcbfl, dcbflp>: Add tests.
	* testsuite/gas/ppc/power9.d: Likewise.
	* testsuite/gas/ppc/power10.s <dcbf, dcbfps, dcbstps, hwsync, lwsync,
	pause_short, phwsync, plwsync, ptesync, stcisync, stncisync, stsync,
	sync, wait, waitrsv>: Add tests.
	* testsuite/gas/ppc/power10.d: Likewise.

diff --git a/opcodes/ppc-opc.c b/opcodes/ppc-opc.c
index ed0a668ad6..ecf526a413 100644
--- a/opcodes/ppc-opc.c
+++ b/opcodes/ppc-opc.c
@@ -835,10 +835,15 @@ extract_li20 (uint64_t insn,
 	   | (insn & 0x7ff)) ^ 0x80000) - 0x80000;
 }
 
-/* The 2-bit L field in a SYNC or WC field in a WAIT instruction.
+/* The 2-bit/3-bit L or 2-bit WC field in a SYNC, DCBF or WAIT instruction.
    For SYNC, some L values are reserved:
-     * Value 3 is reserved on newer server cpus.
-     * Values 2 and 3 are reserved on all other cpus.  */
+     * Values 3, 6 and 7 are reserved on all cpus.
+     * Value 2 is reserved on all other cpus.
+   For DCBF, some L values are reserved:
+     * Values 2, 5 and 7 are reserved on all cpus.
+   For WAIT, some WC values are reserved:
+     * Value 3 is reserved on all server cpus.
+     * Values 1 and 2 are reserved on older server cpus.  */
 
 static uint64_t
 insert_ls (uint64_t insn,
@@ -846,15 +851,73 @@ insert_ls (uint64_t insn,
 	   ppc_cpu_t dialect,
 	   const char **errmsg)
 {
-  /* For SYNC, some L values are illegal.  */
+  int64_t mask;
+
   if (((insn >> 1) & 0x3ff) == 598)
     {
-      int64_t max_lvalue = (dialect & PPC_OPCODE_POWER4) ? 2 : 1;
-      if (value > max_lvalue)
-	*errmsg = _("illegal L operand value");
+      /* For SYNC, some L values are illegal.  */
+      mask = (dialect & PPC_OPCODE_POWER10) ?  0x7 : 0x3;
+
+      /* If the value is within range, check for other illegal values.  */
+      if ((value & mask) == value)
+	switch (value)
+	  {
+	  case 2:
+	    if (dialect & PPC_OPCODE_POWER4)
+	      break;
+	    /* Fall through.  */
+	  case 3:
+	  case 6:
+	  case 7:
+	    *errmsg = _("illegal L operand value");
+	    break;
+	  default:
+	    break;
+	  }
+    }
+  else if (((insn >> 1) & 0x3ff) == 86)
+    {
+      /* For DCBF, some L values are illegal.  */
+      mask = (dialect & PPC_OPCODE_POWER10) ?  0x7 : 0x3;
+
+      /* If the value is within range, check for other illegal values.  */
+      if ((value & mask) == value)
+	switch (value)
+	  {
+	  case 2:
+	  case 5:
+	  case 7:
+	    *errmsg = _("illegal L operand value");
+	    break;
+	  default:
+	    break;
+	  }
+    }
+  else
+    {
+      /* For WAIT, some WC values are illegal.  */
+      mask = 0x3;
+
+      /* If the value is within range, check for other illegal values.  */
+      if ((dialect & PPC_OPCODE_A2) == 0
+	  && (dialect & PPC_OPCODE_E500MC) == 0
+	  && (value & mask) == value)
+	switch (value)
+	  {
+	  case 1:
+	  case 2:
+	    if (dialect & PPC_OPCODE_POWER10)
+	      break;
+	    /* Fall through.  */
+	  case 3:
+	    *errmsg = _("illegal WC operand value");
+	    break;
+	  default:
+	    break;
+	  }
     }
 
-  return insn | ((value & 0x3) << 21);
+  return insn | ((value & mask) << 21);
 }
 
 static int64_t
@@ -862,18 +925,72 @@ extract_ls (uint64_t insn,
 	    ppc_cpu_t dialect,
 	    int *invalid)
 {
+  uint64_t value;
+
   /* Missing optional operands have a value of zero.  */
   if (*invalid < 0)
     return 0;
 
-  uint64_t lvalue = (insn >> 21) & 3;
   if (((insn >> 1) & 0x3ff) == 598)
     {
-      uint64_t max_lvalue = (dialect & PPC_OPCODE_POWER4) ? 2 : 1;
-      if (lvalue > max_lvalue)
-	*invalid = 1;
+      /* For SYNC, some L values are illegal.  */
+      int64_t mask = (dialect & PPC_OPCODE_POWER10) ?  0x7 : 0x3;
+
+      value = (insn >> 21) & mask;
+      switch (value)
+	{
+	case 2:
+	  if (dialect & PPC_OPCODE_POWER4)
+	    break;
+	  /* Fall through.  */
+	case 3:
+	case 6:
+	case 7:
+	  *invalid = 1;
+	  break;
+	default:
+	  break;
+	}
+    }
+  else if (((insn >> 1) & 0x3ff) == 86)
+    {
+      /* For DCBF, some L values are illegal.  */
+      int64_t mask = (dialect & PPC_OPCODE_POWER10) ?  0x7 : 0x3;
+
+      value = (insn >> 21) & mask;
+      switch (value)
+	{
+	case 2:
+	case 5:
+	case 7:
+	  *invalid = 1;
+	  break;
+	default:
+	  break;
+	}
     }
-  return lvalue;
+  else
+    {
+      /* For WAIT, some WC values are illegal.  */
+      value = (insn >> 21) & 0x3;
+      if ((dialect & PPC_OPCODE_A2) == 0
+	  && (dialect & PPC_OPCODE_E500MC) == 0)
+	switch (value)
+	  {
+	  case 1:
+	  case 2:
+	    if (dialect & PPC_OPCODE_POWER10)
+	      break;
+	    /* Fall through.  */
+	  case 3:
+	    *invalid = 1;
+	    break;
+	  default:
+	    break;
+	  }
+    }
+
+  return value;
 }
 
 /* The 4-bit E field in a sync instruction that accepts 2 operands.
@@ -1079,6 +1196,41 @@ extract_nsi (uint64_t insn,
   return -(((insn & 0xffff) ^ 0x8000) - 0x8000);
 }
 
+/* The 2-bit SC field in a SYNC or PL field in a WAIT instruction.
+   For WAIT, some PL values are reserved:
+     * Values 1, 2 and 3 are reserved.  */
+
+static uint64_t
+insert_pl (uint64_t insn,
+	   int64_t value,
+	   ppc_cpu_t dialect ATTRIBUTE_UNUSED,
+	   const char **errmsg)
+{
+  /* For WAIT, some PL values are illegal.  */
+  if (((insn >> 1) & 0x3ff) == 30
+      && value != 0)
+    *errmsg = _("illegal PL operand value");
+  return insn | ((value & 0x3) << 16);
+}
+
+static int64_t
+extract_pl (uint64_t insn,
+	    ppc_cpu_t dialect ATTRIBUTE_UNUSED,
+	    int *invalid)
+{
+  /* Missing optional operands have a value of zero.  */
+  if (*invalid < 0)
+    return 0;
+
+  uint64_t value = (insn >> 16) & 0x3;
+
+  /* For WAIT, some PL values are illegal.  */
+  if (((insn >> 1) & 0x3ff) == 30
+      && value != 0)
+    *invalid = 1;
+  return value;
+}
+
 /* The RA field in a D or X form instruction which is an updating
    load, which means that the RA field may not be zero and may not
    equal the RT field.  */
@@ -2443,9 +2595,11 @@ const struct powerpc_operand powerpc_operands[] =
 #define L32OPT L1OPT + 1
   { 0x1, 21, NULL, NULL, PPC_OPERAND_OPTIONAL | PPC_OPERAND_OPTIONAL32 },
 
-  /* The L field in dcbf instruction.  */
+  /* The 2-bit L or WC field in an X (sync, dcbf or wait) form instruction.  */
 #define L2OPT L32OPT + 1
-  { 0x3, 21, NULL, NULL, PPC_OPERAND_OPTIONAL },
+#define LS L2OPT
+#define WC L2OPT
+  { 0x3, 21, insert_ls, extract_ls, PPC_OPERAND_OPTIONAL },
 
   /* The LEV field in a POWER SVC / POWER9 SCV form instruction.  */
 #define SVC_LEV L2OPT + 1
@@ -2465,13 +2619,13 @@ const struct powerpc_operand powerpc_operands[] =
 #define LIA LI + 1
   { 0x3fffffc, 0, NULL, NULL, PPC_OPERAND_ABSOLUTE | PPC_OPERAND_SIGNED },
 
-  /* The LS or WC field in an X (sync or wait) form instruction.  */
-#define LS LIA + 1
-#define WC LS
-  { 0x3, 21, insert_ls, extract_ls, PPC_OPERAND_OPTIONAL },
+  /* The 3-bit L field in a sync or dcbf instruction.  */
+#define LS3 LIA + 1
+#define L3OPT LS3
+  { 0x7, 21, insert_ls, extract_ls, PPC_OPERAND_OPTIONAL },
 
   /* The ME field in an M form instruction.  */
-#define ME LS + 1
+#define ME LS3 + 1
 #define ME_MASK (0x1f << 1)
   { 0x1f, 1, NULL, NULL, 0 },
 
@@ -3044,8 +3198,13 @@ const struct powerpc_operand powerpc_operands[] =
 #define IH ERAT_T + 1
   { 0x7, 21, NULL, NULL, PPC_OPERAND_OPTIONAL },
 
+  /* The 2-bit SC or PL field in an X form instruction.  */
+#define SC2 IH + 1
+#define PL SC2
+  { 0x3, 16, insert_pl, extract_pl, PPC_OPERAND_OPTIONAL },
+
   /* The 8-bit IMM8 field in a XX1 form instruction.  */
-#define IMM8 IH + 1
+#define IMM8 SC2 + 1
   { 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
 
 #define VX_OFF IMM8 + 1
@@ -3594,6 +3753,10 @@ const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
    field.  */
 #define XWC_MASK (XRC (0x3f, 0x3ff, 1) | (7 << 23) | RA_MASK | RB_MASK)
 
+/* An X form wait instruction with everything filled in except the WC
+   and PL fields.  */
+#define XWCPL_MASK (XRC (0x3f, 0x3ff, 1) | (7 << 23) | (3 << 18) | RB_MASK)
+
 /* The mask for an XX1 form instruction.  */
 #define XX1_MASK X (0x3f, 0x3ff)
 
@@ -3659,9 +3822,12 @@ const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
 /* An X_MASK with the RT field fixed.  */
 #define XRT_MASK (X_MASK | RT_MASK)
 
-/* An XRT_MASK mask with the L bits clear.  */
+/* An XRT_MASK mask with the 2 L bits clear.  */
 #define XLRT_MASK (XRT_MASK & ~((uint64_t) 0x3 << 21))
 
+/* An XRT_MASK mask with the 3 L bits clear.  */
+#define XL3RT_MASK (XRT_MASK & ~((uint64_t) 0x7 << 21))
+
 /* An X_MASK with the RA and RB fields fixed.  */
 #define XRARB_MASK (X_MASK | RA_MASK | RB_MASK)
 
@@ -3700,11 +3866,21 @@ const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
   (X ((op), (xop))				\
    | ((((uint64_t)(l)) & 1) << 21))
 
-/* An X form instruction with the L bits specified.  */
+/* An X form instruction with the 2 L bits specified.  */
 #define XOPL2(op, xop, l)			\
   (X ((op), (xop))				\
    | ((((uint64_t)(l)) & 3) << 21))
 
+/* An X form instruction with the 3 L bits specified.  */
+#define XOPL3(op, xop, l)			\
+  (X ((op), (xop))				\
+   | ((((uint64_t)(l)) & 7) << 21))
+
+/* An X form instruction with the WC and PL bits specified.  */
+#define XWCPL(op, xop, wc, pl)			\
+  (XOPL3 ((op), (xop), (wc))			\
+   | ((((uint64_t)(pl)) & 3) << 16))
+
 /* An X form instruction with the L bit and RC bit specified.  */
 #define XRCL(op, xop, l, rc)			\
   (XRC ((op), (xop), (rc))			\
@@ -3753,6 +3929,16 @@ const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
    and E fields.  */
 #define XSYNCLE_MASK (0xff90ffff)
 
+/* An X form sync instruction.  */
+#define XSYNCLS(op, xop, l, s)			\
+  (X ((op), (xop))				\
+   | ((((uint64_t)(l)) & 7) << 21)		\
+   | ((((uint64_t)(s)) & 3) << 16))
+
+/* An X form sync instruction with everything filled in except the
+   L and SC fields.  */
+#define XSYNCLS_MASK (0xff1cffff)
+
 /* An X_MASK, but with the EH bit clear.  */
 #define XEH_MASK (X_MASK & ~((uint64_t )1))
 
@@ -6076,7 +6262,10 @@ const struct powerpc_opcode powerpc_opcodes[] = {
 {"ldepx",	X(31,29),	X_MASK,	  E500MC|PPCA2, 0,		{RT, RA0, RB}},
 
 {"waitasec",	X(31,30),      XRTRARB_MASK, POWER8,	POWER9,		{0}},
-{"wait",	X(31,30),	XWC_MASK,    POWER9,	0,		{WC}},
+{"waitrsv",	XWCPL(31,30,1,0),0xffffffff, POWER10,	0,		{0}},
+{"pause_short",	XWCPL(31,30,2,0),0xffffffff, POWER10,	0,		{0}},
+{"wait",	X(31,30),	XWCPL_MASK,  POWER10,	0,		{WC, PL}},
+{"wait",	X(31,30),	XWC_MASK,    POWER9,	POWER10,	{WC}},
 
 {"lwepx",	X(31,31),	X_MASK,	  E500MC|PPCA2, 0,		{RT, RA0, RB}},
 
@@ -6174,7 +6363,11 @@ const struct powerpc_opcode powerpc_opcodes[] = {
 {"ldarx",	X(31,84),	XEH_MASK,    PPC64,	0,		{RT, RA0, RB, EH}},
 
 {"dcbfl",	XOPL(31,86,1),	XRT_MASK,    POWER5,	PPC476,		{RA0, RB}},
-{"dcbf",	X(31,86),	XLRT_MASK,   PPC,	0,		{RA0, RB, L2OPT}},
+{"dcbflp",	XOPL2(31,86,3), XRT_MASK,    POWER9,	PPC476,		{RA0, RB}},
+{"dcbfps",	XOPL3(31,86,4), XRT_MASK,    POWER10,   PPC476,		{RA0, RB}},
+{"dcbstps",	XOPL3(31,86,6), XRT_MASK,    POWER10,   PPC476,		{RA0, RB}},
+{"dcbf",	X(31,86),	XL3RT_MASK,  POWER10,	PPC476,		{RA0, RB, L3OPT}},
+{"dcbf",	X(31,86),	XLRT_MASK,   PPC,	POWER10,	{RA0, RB, L2OPT}},
 
 {"lbzx",	X(31,87),	X_MASK,	     COM,	0,		{RT, RA0, RB}},
 
@@ -7243,8 +7436,14 @@ const struct powerpc_opcode powerpc_opcodes[] = {
 {"hwsync",	XSYNC(31,598,0), 0xffffffff, POWER4,	BOOKE|PPC476,	{0}},
 {"lwsync",	XSYNC(31,598,1), 0xffffffff, PPC,	E500,		{0}},
 {"ptesync",	XSYNC(31,598,2), 0xffffffff, PPC64,	0,		{0}},
+{"phwsync",	XSYNCLS(31,598,4,0), 0xffffffff, POWER10, 0,		{0}},
+{"plwsync",	XSYNCLS(31,598,5,0), 0xffffffff, POWER10, 0,		{0}},
+{"stncisync",	XSYNCLS(31,598,1,1), 0xffffffff, POWER10, 0,		{0}},
+{"stcisync",	XSYNCLS(31,598,0,2), 0xffffffff, POWER10, 0,		{0}},
+{"stsync",	XSYNCLS(31,598,0,3), 0xffffffff, POWER10, 0,		{0}},
+{"sync",	X(31,598),     XSYNCLS_MASK, POWER10,	BOOKE|PPC476,	{LS3, SC2}},
 {"sync",	X(31,598),     XSYNCLE_MASK, E6500,	0,		{LS, ESYNC}},
-{"sync",	X(31,598),     XSYNC_MASK,   PPCCOM,	BOOKE|PPC476,	{LS}},
+{"sync",	X(31,598),     XSYNC_MASK,   PPCCOM,	POWER10|BOOKE|PPC476, {LS}},
 {"msync",	X(31,598),     0xffffffff, BOOKE|PPCA2|PPC476, 0,	{0}},
 {"sync",	X(31,598),     0xffffffff,   BOOKE|PPC476, E6500,	{0}},
 {"lwsync",	X(31,598),     0xffffffff,   E500,	0,		{0}},
diff --git a/gas/testsuite/gas/ppc/power9.s b/gas/testsuite/gas/ppc/power9.s
index 1e9266cf9f..69053819ff 100644
--- a/gas/testsuite/gas/ppc/power9.s
+++ b/gas/testsuite/gas/ppc/power9.s
@@ -384,3 +384,9 @@ power9:
 	scv         0
 	scv         127
 	rfscv
+	dcbf	    0,3
+	dcbf	    0,3,0
+	dcbfl	    0,4
+	dcbf	    0,4,1
+	dcbflp	    0,5
+	dcbf	    0,5,3
diff --git a/gas/testsuite/gas/ppc/power9.d b/gas/testsuite/gas/ppc/power9.d
index 64cfb09197..4e7156d46d 100644
--- a/gas/testsuite/gas/ppc/power9.d
+++ b/gas/testsuite/gas/ppc/power9.d
@@ -393,4 +393,10 @@ Disassembly of section \.text:
 .*:	(01 00 00 44|44 00 00 01) 	scv     0
 .*:	(e1 0f 00 44|44 00 0f e1) 	scv     127
 .*:	(a4 00 00 4c|4c 00 00 a4) 	rfscv
+.*:	(7c 00 18 ac|ac 18 00 7c) 	dcbf    0,r3
+.*:	(7c 00 18 ac|ac 18 00 7c) 	dcbf    0,r3
+.*:	(7c 20 20 ac|ac 20 20 7c) 	dcbfl   0,r4
+.*:	(7c 20 20 ac|ac 20 20 7c) 	dcbfl   0,r4
+.*:	(7c 60 28 ac|ac 28 60 7c) 	dcbflp  0,r5
+.*:	(7c 60 28 ac|ac 28 60 7c) 	dcbflp  0,r5
 #pass
diff --git a/gas/testsuite/gas/ppc/power10.s b/gas/testsuite/gas/ppc/power10.s
index 116487c276..79893b966b 100644
--- a/gas/testsuite/gas/ppc/power10.s
+++ b/gas/testsuite/gas/ppc/power10.s
@@ -6,3 +6,38 @@ _start:
 	paste.  10,11
 	paste.  10,11,1
 	paste.  10,11,0
+	dcbfps	0,3
+	dcbf	0,3,4
+	dcbstps	0,3
+	dcbf	0,3,6
+	hwsync
+	sync
+	sync 0
+	sync 0,0
+	lwsync
+	sync 1
+	sync 1,0
+	ptesync
+	sync 2
+	sync 2,0
+	phwsync
+	sync 4
+	sync 4,0
+	plwsync
+	sync 5
+	sync 5,0
+	stncisync
+	sync 1,1
+	stcisync
+	sync 0,2
+	stsync
+	sync 0,3
+	wait
+	wait 0
+	wait 0,0
+	waitrsv
+	wait 1
+	wait 1,0
+	pause_short
+	wait 2
+	wait 2,0
diff --git a/gas/testsuite/gas/ppc/power10.d b/gas/testsuite/gas/ppc/power10.d
index 3fc4b4fb43..efa5be47ec 100644
--- a/gas/testsuite/gas/ppc/power10.d
+++ b/gas/testsuite/gas/ppc/power10.d
@@ -13,4 +13,39 @@ Disassembly of section \.text:
 .*:	(7c 2a 5f 0d|0d 5f 2a 7c) 	paste\.  r10,r11
 .*:	(7c 2a 5f 0d|0d 5f 2a 7c) 	paste\.  r10,r11
 .*:	(7c 0a 5f 0d|0d 5f 0a 7c) 	paste\.  r10,r11,0
+.*:	(7c 80 18 ac|ac 18 80 7c) 	dcbfps  0,r3
+.*:	(7c 80 18 ac|ac 18 80 7c) 	dcbfps  0,r3
+.*:	(7c c0 18 ac|ac 18 c0 7c) 	dcbstps 0,r3
+.*:	(7c c0 18 ac|ac 18 c0 7c) 	dcbstps 0,r3
+.*:	(7c 00 04 ac|ac 04 00 7c) 	hwsync
+.*:	(7c 00 04 ac|ac 04 00 7c) 	hwsync
+.*:	(7c 00 04 ac|ac 04 00 7c) 	hwsync
+.*:	(7c 00 04 ac|ac 04 00 7c) 	hwsync
+.*:	(7c 20 04 ac|ac 04 20 7c) 	lwsync
+.*:	(7c 20 04 ac|ac 04 20 7c) 	lwsync
+.*:	(7c 20 04 ac|ac 04 20 7c) 	lwsync
+.*:	(7c 40 04 ac|ac 04 40 7c) 	ptesync
+.*:	(7c 40 04 ac|ac 04 40 7c) 	ptesync
+.*:	(7c 40 04 ac|ac 04 40 7c) 	ptesync
+.*:	(7c 80 04 ac|ac 04 80 7c) 	phwsync
+.*:	(7c 80 04 ac|ac 04 80 7c) 	phwsync
+.*:	(7c 80 04 ac|ac 04 80 7c) 	phwsync
+.*:	(7c a0 04 ac|ac 04 a0 7c) 	plwsync
+.*:	(7c a0 04 ac|ac 04 a0 7c) 	plwsync
+.*:	(7c a0 04 ac|ac 04 a0 7c) 	plwsync
+.*:	(7c 21 04 ac|ac 04 21 7c) 	stncisync
+.*:	(7c 21 04 ac|ac 04 21 7c) 	stncisync
+.*:	(7c 02 04 ac|ac 04 02 7c) 	stcisync
+.*:	(7c 02 04 ac|ac 04 02 7c) 	stcisync
+.*:	(7c 03 04 ac|ac 04 03 7c) 	stsync
+.*:	(7c 03 04 ac|ac 04 03 7c) 	stsync
+.*:	(7c 00 00 3c|3c 00 00 7c) 	wait
+.*:	(7c 00 00 3c|3c 00 00 7c) 	wait
+.*:	(7c 00 00 3c|3c 00 00 7c) 	wait
+.*:	(7c 20 00 3c|3c 00 20 7c) 	waitrsv
+.*:	(7c 20 00 3c|3c 00 20 7c) 	waitrsv
+.*:	(7c 20 00 3c|3c 00 20 7c) 	waitrsv
+.*:	(7c 40 00 3c|3c 00 40 7c) 	pause_short
+.*:	(7c 40 00 3c|3c 00 40 7c) 	pause_short
+.*:	(7c 40 00 3c|3c 00 40 7c) 	pause_short
 #pass

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Alan Modra]

On Sat, May 16, 2020 at 05:53:20PM -0500, Peter Bergner wrote:
> -/* The 2-bit L field in a SYNC or WC field in a WAIT instruction.
> +/* The 2-bit/3-bit L or 2-bit WC field in a SYNC, DCBF or WAIT instruction.
>     For SYNC, some L values are reserved:
> -     * Value 3 is reserved on newer server cpus.
> -     * Values 2 and 3 are reserved on all other cpus.  */
> +     * Values 3, 6 and 7 are reserved on all cpus.
> +     * Value 2 is reserved on all other cpus.

The above needs fixing.

> +      /* For SYNC, some L values are illegal.  */
> +      mask = (dialect & PPC_OPCODE_POWER10) ?  0x7 : 0x3;
> +
> +      /* If the value is within range, check for other illegal values.  */
> +      if ((value & mask) == value)
> +	switch (value)
> +	  {
> +	  case 2:
> +	    if (dialect & PPC_OPCODE_POWER4)
> +	      break;
> +	    /* Fall through.  */
> +	  case 3:
> +	  case 6:
> +	  case 7:
> +	    *errmsg = _("illegal L operand value");
> +	    break;
> +	  default:
> +	    break;
> +	  }

So if "(value & mask) != value" then no checks are done?  That doesn't
seem correct.  The same problem occurs later too.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Peter Bergner]

On 5/17/20 9:36 PM, Alan Modra wrote:
> On Sat, May 16, 2020 at 05:53:20PM -0500, Peter Bergner wrote:
>> -/* The 2-bit L field in a SYNC or WC field in a WAIT instruction.
>> +/* The 2-bit/3-bit L or 2-bit WC field in a SYNC, DCBF or WAIT instruction.
>>     For SYNC, some L values are reserved:
>> -     * Value 3 is reserved on newer server cpus.
>> -     * Values 2 and 3 are reserved on all other cpus.  */
>> +     * Values 3, 6 and 7 are reserved on all cpus.
>> +     * Value 2 is reserved on all other cpus.
> 
> The above needs fixing.

How so?


> So if "(value & mask) != value" then no checks are done?  That doesn't
> seem correct.  The same problem occurs later too.

Well, no checks are done here.  If (value & mask) != value, then
we've already triggered an operand out of range bug and there is no
need for another test here.  It's only if the value fits within the
mask range that we need to check for illegal values within the mask
range.  If we didn't do this, we'd sometimes end up with an operand
out of range error and a possible illegal value error.

Fr example, notice below how line 5 is an illegal value for power10,
but an out of range error for power9 and earlier.

Peter


bergner@pike:~$ cat -n sync.s 
     1		.text
     2	_start:
     3		sync 2
     4		sync 4
     5		sync 6
bergner@pike:~$ /home/bergner/binutils/build/binutils-p10/gas/as-new -mpower10 sync.s 
sync.s: Assembler messages:
sync.s:5: Error: illegal L operand value
bergner@pike:~$ /home/bergner/binutils/build/binutils-p10/gas/as-new -mpower9 sync.s 
sync.s: Assembler messages:
sync.s:4: Error: operand out of range (4 is not between 0 and 3)
sync.s:5: Error: operand out of range (6 is not between 0 and 3)
bergner@pike:~$ /home/bergner/binutils/build/binutils-p10/gas/as-new -mcom sync.s 
sync.s: Assembler messages:
sync.s:3: Error: illegal L operand value
sync.s:4: Error: operand out of range (4 is not between 0 and 3)
sync.s:5: Error: operand out of range (6 is not between 0 and 3)

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Alan Modra]

On Sun, May 17, 2020 at 10:36:34PM -0500, Peter Bergner wrote:
> On 5/17/20 9:36 PM, Alan Modra wrote:
> > On Sat, May 16, 2020 at 05:53:20PM -0500, Peter Bergner wrote:
> >> -/* The 2-bit L field in a SYNC or WC field in a WAIT instruction.
> >> +/* The 2-bit/3-bit L or 2-bit WC field in a SYNC, DCBF or WAIT instruction.
> >>     For SYNC, some L values are reserved:
> >> -     * Value 3 is reserved on newer server cpus.
> >> -     * Values 2 and 3 are reserved on all other cpus.  */
> >> +     * Values 3, 6 and 7 are reserved on all cpus.
> >> +     * Value 2 is reserved on all other cpus.
> > 
> > The above needs fixing.
> 
> How so?

You say "3, 6 and 7 are reserved on all cpus".  That doesn't leave
much room for "other cpus".

> 
> > So if "(value & mask) != value" then no checks are done?  That doesn't
> > seem correct.  The same problem occurs later too.
> 
> Well, no checks are done here.  If (value & mask) != value, then
> we've already triggered an operand out of range bug and there is no
> need for another test here.  It's only if the value fits within the
> mask range that we need to check for illegal values within the mask
> range.  If we didn't do this, we'd sometimes end up with an operand
> out of range error and a possible illegal value error.
> 
> Fr example, notice below how line 5 is an illegal value for power10,
> but an out of range error for power9 and earlier.

Oh of course.  Not enough coffee or something.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Peter Bergner]

On 5/17/20 11:49 PM, Alan Modra wrote:
> On Sun, May 17, 2020 at 10:36:34PM -0500, Peter Bergner wrote:
>> On 5/17/20 9:36 PM, Alan Modra wrote:
>>> On Sat, May 16, 2020 at 05:53:20PM -0500, Peter Bergner wrote:
>>>> -/* The 2-bit L field in a SYNC or WC field in a WAIT instruction.
>>>> +/* The 2-bit/3-bit L or 2-bit WC field in a SYNC, DCBF or WAIT instruction.
>>>>     For SYNC, some L values are reserved:
>>>> -     * Value 3 is reserved on newer server cpus.
>>>> -     * Values 2 and 3 are reserved on all other cpus.  */
>>>> +     * Values 3, 6 and 7 are reserved on all cpus.
>>>> +     * Value 2 is reserved on all other cpus.
>>>
>>> The above needs fixing.
>>
>> How so?
> 
> You say "3, 6 and 7 are reserved on all cpus".  That doesn't leave
> much room for "other cpus".

Well it's true that no cpus support 3, 6 and 7.  Although 6 and 7 are
out of range for cpus before power10.  Values 4 and 5 are the new
values allowed for power10 which are out of range for all other cpus.

Do you want:

	* Values 3, 6 and 7 are reserved on newer server cpus.
	* Value 2 is reserved on all other cpus.

...or this?

	* Values 6 and 7 are reserved on newer server cpus.
	* Value 3 is reserved on older server cpus.
	* Value 2 is reserved on all other cpus.

Or do you have something else in mind?

Peter

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Peter Bergner]

On 5/18/20 10:02 AM, Peter Bergner wrote:
> 	* Values 6 and 7 are reserved on newer server cpus.
> 	* Value 3 is reserved on older server cpus.
> 	* Value 2 is reserved on all other cpus.

	* Values 6 and 7 are reserved on newer server cpus.
 	* Value 3 is reserved on all server cpus.
 	* Value 2 is reserved on all other cpus.

...or maybe this version should be restated as above?

Peter

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Tulio Magno Quites Machado Filho]

Peter Bergner via Binutils <binutils@sourceware.org> writes:

> @@ -6174,7 +6363,11 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>  {"ldarx",	X(31,84),	XEH_MASK,    PPC64,	0,		{RT, RA0, RB, EH}},
>  
>  {"dcbfl",	XOPL(31,86,1),	XRT_MASK,    POWER5,	PPC476,		{RA0, RB}},
> -{"dcbf",	X(31,86),	XLRT_MASK,   PPC,	0,		{RA0, RB, L2OPT}},
> +{"dcbflp",	XOPL2(31,86,3), XRT_MASK,    POWER9,	PPC476,		{RA0, RB}},
> +{"dcbfps",	XOPL3(31,86,4), XRT_MASK,    POWER10,   PPC476,		{RA0, RB}},
> +{"dcbstps",	XOPL3(31,86,6), XRT_MASK,    POWER10,   PPC476,		{RA0, RB}},
> +{"dcbf",	X(31,86),	XL3RT_MASK,  POWER10,	PPC476,		{RA0, RB, L3OPT}},
> +{"dcbf",	X(31,86),	XLRT_MASK,   PPC,	POWER10,	{RA0, RB, L2OPT}},

Power ISA 3.1 states the following (page 1038):

    The encodings for dcbfps, dcbstps, plwsync, and phwsync were chosen to
    enable software developed to control updates to persistent storage on
    processors that comply with Version 3.1 and subsequent versions of the
    architecture to run compatibly on the POWER9 processor.

So, I think these instructions should be available on POWER9 too.

> @@ -7243,8 +7436,14 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>  {"hwsync",	XSYNC(31,598,0), 0xffffffff, POWER4,	BOOKE|PPC476,	{0}},
>  {"lwsync",	XSYNC(31,598,1), 0xffffffff, PPC,	E500,		{0}},
>  {"ptesync",	XSYNC(31,598,2), 0xffffffff, PPC64,	0,		{0}},
> +{"phwsync",	XSYNCLS(31,598,4,0), 0xffffffff, POWER10, 0,		{0}},
> +{"plwsync",	XSYNCLS(31,598,5,0), 0xffffffff, POWER10, 0,		{0}},
> +{"stncisync",	XSYNCLS(31,598,1,1), 0xffffffff, POWER10, 0,		{0}},
> +{"stcisync",	XSYNCLS(31,598,0,2), 0xffffffff, POWER10, 0,		{0}},
> +{"stsync",	XSYNCLS(31,598,0,3), 0xffffffff, POWER10, 0,		{0}},
> +{"sync",	X(31,598),     XSYNCLS_MASK, POWER10,	BOOKE|PPC476,	{LS3, SC2}},

Likewise here.
Power ISA 3.1, page 1087, figure 5 better describes how they're executed on
earlier processors.

-- 
Tulio Magno

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Peter Bergner]

On 5/18/20 3:03 PM, Tulio Magno Quites Machado Filho wrote:
> Peter Bergner via Binutils <binutils@sourceware.org> writes:
>> +{"dcbflp",	XOPL2(31,86,3), XRT_MASK,    POWER9,	PPC476,		{RA0, RB}},
>> +{"dcbfps",	XOPL3(31,86,4), XRT_MASK,    POWER10,   PPC476,		{RA0, RB}},
>> +{"dcbstps",	XOPL3(31,86,6), XRT_MASK,    POWER10,   PPC476,		{RA0, RB}},
>> +{"dcbf",	X(31,86),	XL3RT_MASK,  POWER10,	PPC476,		{RA0, RB, L3OPT}},
>> +{"dcbf",	X(31,86),	XLRT_MASK,   PPC,	POWER10,	{RA0, RB, L2OPT}},
> 
> Power ISA 3.1 states the following (page 1038):
> 
>     The encodings for dcbfps, dcbstps, plwsync, and phwsync were chosen to
>     enable software developed to control updates to persistent storage on
>     processors that comply with Version 3.1 and subsequent versions of the
>     architecture to run compatibly on the POWER9 processor.
> 
> So, I think these instructions should be available on POWER9 too.
[snip]
> Likewise here.
> Power ISA 3.1, page 1087, figure 5 better describes how they're executed on
> earlier processors.

I don't think this is saying we should enable the new mnemonics on POWER9,
but rather if one of these is assembled for POWER10, that they'll run on
POWER9 just fine.  Ditto for the sync insns.

Alan or Bill,

How do you read those paragraphs?

Peter

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Alan Modra]

On Mon, May 18, 2020 at 10:45:23AM -0500, Peter Bergner wrote:
> On 5/18/20 10:02 AM, Peter Bergner wrote:
> > 	* Values 6 and 7 are reserved on newer server cpus.
> > 	* Value 3 is reserved on older server cpus.
> > 	* Value 2 is reserved on all other cpus.
> 
> 	* Values 6 and 7 are reserved on newer server cpus.
>  	* Value 3 is reserved on all server cpus.
>  	* Value 2 is reserved on all other cpus.
> 
> ...or maybe this version should be restated as above?

That looks good.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Alan Modra]

On Mon, May 18, 2020 at 04:40:24PM -0500, Peter Bergner wrote:
> On 5/18/20 3:03 PM, Tulio Magno Quites Machado Filho wrote:
> > Power ISA 3.1 states the following (page 1038):
> > 
> >     The encodings for dcbfps, dcbstps, plwsync, and phwsync were chosen to
> >     enable software developed to control updates to persistent storage on
> >     processors that comply with Version 3.1 and subsequent versions of the
> >     architecture to run compatibly on the POWER9 processor.
> > 
> > So, I think these instructions should be available on POWER9 too.
> [snip]
> > Likewise here.
> > Power ISA 3.1, page 1087, figure 5 better describes how they're executed on
> > earlier processors.
> 
> I don't think this is saying we should enable the new mnemonics on POWER9,
> but rather if one of these is assembled for POWER10, that they'll run on
> POWER9 just fine.  Ditto for the sync insns.
> 
> Alan or Bill,
> 
> How do you read those paragraphs?

With the same conclusion as you do.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Bill Schmidt]

On 5/18/20 8:04 PM, Alan Modra wrote:
> On Mon, May 18, 2020 at 04:40:24PM -0500, Peter Bergner wrote:
>> On 5/18/20 3:03 PM, Tulio Magno Quites Machado Filho wrote:
>>> Power ISA 3.1 states the following (page 1038):
>>>
>>>      The encodings for dcbfps, dcbstps, plwsync, and phwsync were chosen to
>>>      enable software developed to control updates to persistent storage on
>>>      processors that comply with Version 3.1 and subsequent versions of the
>>>      architecture to run compatibly on the POWER9 processor.
>>>
>>> So, I think these instructions should be available on POWER9 too.
>> [snip]
>>> Likewise here.
>>> Power ISA 3.1, page 1087, figure 5 better describes how they're executed on
>>> earlier processors.
>> I don't think this is saying we should enable the new mnemonics on POWER9,
>> but rather if one of these is assembled for POWER10, that they'll run on
>> POWER9 just fine.  Ditto for the sync insns.
>>
>> Alan or Bill,
>>
>> How do you read those paragraphs?
> With the same conclusion as you do.
>
Likewise.

Re: PowerPC POWER10 updates to dcbf, sync and wait instructions

[Peter Bergner]

On 5/18/20 10:45 AM, Peter Bergner wrote:
> On 5/18/20 10:02 AM, Peter Bergner wrote:
>> 	* Values 6 and 7 are reserved on newer server cpus.
>> 	* Value 3 is reserved on older server cpus.
>> 	* Value 2 is reserved on all other cpus.
> 
> 	* Values 6 and 7 are reserved on newer server cpus.
>  	* Value 3 is reserved on all server cpus.
>  	* Value 2 is reserved on all other cpus.
> 
> ...or maybe this version should be restated as above?

Ok, I pushed the patch with these changes.  Thanks.

Peter