Re: [PATCH, AARCH64] support s2_<op1>_<Cn>_<Cm>_<op2> sys reg

[Zhenqiang Chen <zhenqiang dot chen at linaro dot org>]

On 13 November 2013 07:09, Yufeng Zhang <Yufeng.Zhang@arm.com> wrote:
> Hi Zhenqiang,
>
>
> On 11/12/13 06:12, Zhenqiang Chen wrote:
>>
>> Hi,
>>
>> Current parse_sys_reg only supports s3_<op1>_<Cn>_<Cm>_<op2>. But
>> according to the document,
>>
>>    integer sys_op0 = 2 + UInt(o0)
>>
>> o0 is bit-19.
>>
>> So sys_op0 can be 2 or 3.
>>
>> This patch changes GAS to to add s2_<op1>_<Cn>_<Cm>_<op2>  support.
>> OK for trunk?
>
>
> For MSR/MRS instruction, it is correct that sys_op0 can be 2 or 3. However,
> it doesn't necessarily mean that the encoding space of sys_op0==2 contains
> reserved area for implementation defined system registers.
>
> The ARMARMv8 mentions in section C4.2.6 "Op0==0b11, Moves to and from
> non-debug System registers and special-purpose registers" that:
>
> "The instructions that move data to and from non-debug system registers are
> encoded with Op0==0b11, except that some of this encoding space is reserved
> for IMPLEMENTATION DEFINED functionality."
>
> The document however doesn't mention about the implementation defined
> functionality in section C4.2.5 "Op0==0b10, Moves to and from debug, qtrace,
> and Execution environment System registers".

C4.2.5
...
This section includes **only** the System register access encodings
for which both:
* Op0 is 0b10.
* The value of Op1 is one of {0, 3, 4}.

So this section only covers "Debug" registers, not "Trace" and
"Execution environment" registers.

Section C5.6.129 defines the full encode.

> Is there any named system register in the op0==0b10 encoding space that you
> would like to add support for, by extending the syntax?

I do not know. Maybe customers can define some registers and map them
to s2_<op1>_<Cn>_<Cm>_<op2>.

> Also if we go head with the patch, please also add some tests to
> gas/testsuite/gas/aarch64/sysreg.[d|s].

Thanks. Patch is updated to include tests.

-Zhenqiang

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 14ffdad..e992e7c 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -3270,7 +3270,7 @@ parse_barrier (char **str)
    Returns the encoding for the option, or PARSE_FAIL.
 
    If IMPLE_DEFINED_P is non-zero, the function will also try to parse the
-   implementation defined system register name S3_<op1>_<Cn>_<Cm>_<op2>.  */
+   implementation defined system register name S<op0>_<op1>_<Cn>_<Cm>_<op2>.  */
 
 static int
 parse_sys_reg (char **str, struct hash_control *sys_regs, int imple_defined_p)
@@ -3295,7 +3295,7 @@ parse_sys_reg (char **str, struct hash_control *sys_regs, int imple_defined_p)
 	return PARSE_FAIL;
       else
 	{
-	  /* Parse S3_<op1>_<Cn>_<Cm>_<op2>, the implementation defined
+	  /* Parse S<op0>_<op1>_<Cn>_<Cm>_<op2>, the implementation defined
 	     registers.  */
 	  unsigned int op0, op1, cn, cm, op2;
 	  if (sscanf (buf, "s%u_%u_c%u_c%u_%u", &op0, &op1, &cn, &cm, &op2) != 5)
@@ -3303,11 +3303,11 @@ parse_sys_reg (char **str, struct hash_control *sys_regs, int imple_defined_p)
 	  /* The architecture specifies the encoding space for implementation
 	     defined registers as:
 	     op0  op1  CRn   CRm   op2
-	     11   xxx  1x11  xxxx  xxx
+	     1x   xxx  1x11  xxxx  xxx
 	     For convenience GAS accepts a wider encoding space, as follows:
 	     op0  op1  CRn   CRm   op2
-	     11   xxx  xxxx  xxxx  xxx  */
-	  if (op0 != 3 || op1 > 7 || cn > 15 || cm > 15 || op2 > 7)
+	     1x   xxx  xxxx  xxxx  xxx  */
+	  if (op0 != 2 && op0 != 3 || op1 > 7 || cn > 15 || cm > 15 || op2 > 7)
 	    return PARSE_FAIL;
 	  value = (op0 << 14) | (op1 << 11) | (cn << 7) | (cm << 3) | op2;
 	}
diff --git a/gas/testsuite/gas/aarch64/sysreg.d b/gas/testsuite/gas/aarch64/sysreg.d
index c7cf00e..7795b4d 100644
--- a/gas/testsuite/gas/aarch64/sysreg.d
+++ b/gas/testsuite/gas/aarch64/sysreg.d
@@ -26,3 +26,5 @@ Disassembly of section \.text:
   48:	d538cc00 	mrs	x0, s3_0_c12_c12_0
   4c:	d5384600 	mrs	x0, s3_0_c4_c6_0
   50:	d5184600 	msr	s3_0_c4_c6_0, x0
+  54:	d5310300 	mrs	x0, s2_1_c0_c3_0
+  58:	d5110300 	msr	s2_1_c0_c3_0, x0
diff --git a/gas/testsuite/gas/aarch64/sysreg.s b/gas/testsuite/gas/aarch64/sysreg.s
index 3287594..b7e5ff6 100644
--- a/gas/testsuite/gas/aarch64/sysreg.s
+++ b/gas/testsuite/gas/aarch64/sysreg.s
@@ -26,3 +26,6 @@
 	mrs x0, s3_0_c12_c12_0
 	mrs x0, s3_0_c4_c6_0
 	msr s3_0_c4_c6_0, x0
+
+	mrs x0, s2_1_c0_c3_0
+	msr s2_1_c0_c3_0, x0