public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] AArch64: add GAS support for UDF instruction
@ 2020-04-28 10:42 Alex Coplan
  2020-04-29  7:44 ` Tamar Christina
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Coplan @ 2020-04-28 10:42 UTC (permalink / raw)
  To: binutils; +Cc: nd

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

Hello,

The attached patch adds support to the AArch64 assembler for the UDF instruction, see
https://developer.arm.com/docs/ddi0596/a/a64-base-instructions-alphabetic-order/udf-permanently-undefined

Note that the files marked regenerated are not included in the patch in order to
keep the size of the patch down: these will need to be regenerated before
commit.

Testing:
 - New regression test added: fails before and passes after the patch.
 - Bootstrap and regression on aarch64-linux.

Ok for master? If so, I will need a maintainer to regenerate the generated files
and commit the patch as I don't have commit access.

Thanks,
Alex

---

binutils/ChangeLog:

2020-04-27  Alex Coplan  <alex.coplan@arm.com>

        * testsuite/binutils-all/aarch64/in-order-all.d: Update to use new
          disassembly.
        * testsuite/binutils-all/aarch64/out-of-order-all.d: Likewise.

ld/ChangeLog:

2020-04-28  Alex Coplan  <alex.coplan@arm.com>

        * testsuite/ld-aarch64/erratum843419_tls_ie.d: Use udf in disassembly.
        * testsuite/ld-aarch64/farcall-b-section.d: Likewise.
        * testsuite/ld-aarch64/farcall-back.d: Likewise.
        * testsuite/ld-aarch64/farcall-bl-section.d: Likewise.

gas/ChangeLog:

2020-04-27  Alex Coplan  <alex.coplan@arm.com>

        * config/tc-aarch64.c (fix_insn): Implement for AARCH64_OPND_UNDEFINED.
        * config/tc-aarch64.c (parse_operands): Likewise.
        * testsuite/gas/aarch64/udf.s: New.
        * testsuite/gas/aarch64/udf.d: New.

include/ChangeLog:

2020-04-27  Alex Coplan  <alex.coplan@arm.com>

        * opcode/aarch64.h (enum aarch64_opnd): Add AARCH64_OPND_UNDEFINED.

opcodes/ChangeLog:

2020-04-27  Alex Coplan  <alex.coplan@arm.com>

        * aarch64-opc.h (enum aarch64_field_kind): Add FLD_imm16_2.
        * aarch64-opc.c (fields): Add entry for FLD_imm16_2.
        * aarch64-tbl.h (aarch64_opcode_table): Add udf instruction, entry for
          FLD_imm16_2.
        * aarch64-asm-2.c: Regenerated.
        * aarch64-dis-2.c: Regenerated.
        * aarch64-opc-2.c: Regenerated.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 8938 bytes --]

diff --git a/binutils/testsuite/binutils-all/aarch64/in-order-all.d b/binutils/testsuite/binutils-all/aarch64/in-order-all.d
index a484ca7..a6daea8 100644
--- a/binutils/testsuite/binutils-all/aarch64/in-order-all.d
+++ b/binutils/testsuite/binutils-all/aarch64/in-order-all.d
@@ -10,7 +10,7 @@ Disassembly of section \.func1:
 
 .+ <v1>:
 [^:]+:	8b010000 	add	x0, x0, x1
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section \.func2:
 
@@ -25,12 +25,12 @@ Disassembly of section \.func3:
 [^:]+:	8b010000 	add	x0, x0, x1
 [^:]+:	8b010000 	add	x0, x0, x1
 [^:]+:	8b010000 	add	x0, x0, x1
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section \.rodata:
 
 .+ <\.rodata>:
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section .global:
 
diff --git a/binutils/testsuite/binutils-all/aarch64/out-of-order-all.d b/binutils/testsuite/binutils-all/aarch64/out-of-order-all.d
index d3aa79e..955d190 100644
--- a/binutils/testsuite/binutils-all/aarch64/out-of-order-all.d
+++ b/binutils/testsuite/binutils-all/aarch64/out-of-order-all.d
@@ -20,7 +20,7 @@ Disassembly of section \.func1:
 
 .+ <v1>:
 [^:]+:	8b010000 	add	x0, x0, x1
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section \.func3:
 
@@ -30,9 +30,9 @@ Disassembly of section \.func3:
 [^:]+:	8b010000 	add	x0, x0, x1
 [^:]+:	8b010000 	add	x0, x0, x1
 [^:]+:	8b010000 	add	x0, x0, x1
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section \.rodata:
 
 .+ <\.rodata>:
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 69ccc59..da786ba 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -6149,6 +6149,7 @@ parse_operands (char *str, const aarch64_opcode *opcode)
 	  break;
 
 	case AARCH64_OPND_EXCEPTION:
+	case AARCH64_OPND_UNDEFINED:
 	  po_misc_or_fail (parse_immediate_expression (&str, &inst.reloc.exp,
 						       imm_reg_type));
 	  assign_imm_if_const_or_fixup_later (&inst.reloc, info,
@@ -7745,11 +7746,12 @@ fix_insn (fixS *fixP, uint32_t flags, offsetT value)
   switch (opnd)
     {
     case AARCH64_OPND_EXCEPTION:
+    case AARCH64_OPND_UNDEFINED:
       if (unsigned_overflow (value, 16))
 	as_bad_where (fixP->fx_file, fixP->fx_line,
 		      _("immediate out of range"));
       insn = get_aarch64_insn (buf);
-      insn |= encode_svc_imm (value);
+      insn |= (opnd == AARCH64_OPND_EXCEPTION) ? encode_svc_imm (value) : value;
       put_aarch64_insn (buf, insn);
       break;
 
diff --git a/gas/testsuite/gas/aarch64/udf.d b/gas/testsuite/gas/aarch64/udf.d
new file mode 100644
index 0000000..ae6432b
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/udf.d
@@ -0,0 +1,9 @@
+#objdump: -dr
+
+.*:     file format .*
+
+Disassembly of section \.text:
+
+0+ <.*>:
+.*:	0000002a 	udf	#42
+.*:	0000ffff 	udf	#65535
diff --git a/gas/testsuite/gas/aarch64/udf.s b/gas/testsuite/gas/aarch64/udf.s
new file mode 100644
index 0000000..fec30a3
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/udf.s
@@ -0,0 +1,5 @@
+// Test file for AArch64 udf.
+
+.text
+udf #42
+udf #65535
diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h
index c152952..817ca1e 100644
--- a/include/opcode/aarch64.h
+++ b/include/opcode/aarch64.h
@@ -272,6 +272,7 @@ enum aarch64_opnd
   AARCH64_OPND_UIMM10,	/* Unsigned 10-bit immediate in addg/subg.  */
   AARCH64_OPND_BIT_NUM,	/* Immediate.  */
   AARCH64_OPND_EXCEPTION,/* imm16 operand in exception instructions.  */
+  AARCH64_OPND_UNDEFINED,/* imm16 operand in undefined instruction. */
   AARCH64_OPND_CCMP_IMM,/* Immediate in conditional compare instructions.  */
   AARCH64_OPND_SIMM5,	/* 5-bit signed immediate in the imm5 field.  */
   AARCH64_OPND_NZCV,	/* Flag bit specifier giving an alternative value for
diff --git a/ld/testsuite/ld-aarch64/erratum843419_tls_ie.d b/ld/testsuite/ld-aarch64/erratum843419_tls_ie.d
index eba5a20..4d2b111 100644
--- a/ld/testsuite/ld-aarch64/erratum843419_tls_ie.d
+++ b/ld/testsuite/ld-aarch64/erratum843419_tls_ie.d
@@ -23,10 +23,10 @@ Disassembly of section .e843419:
 [ ]*20001010:	0b0700e0 	add	w0, w7, w7
 [ ]*20001014:	910043ff 	add	sp, sp, #0x10
 [ ]*20001018:	d65f03c0 	ret
-[ ]*2000101c:	00000000 	.inst	0x00000000 ; undefined
+[ ]*2000101c:	00000000 	udf	#0
 [ ]*20001020:	14000400 	b	20002020 <e843419\+0x1028>
 [ ]*20001024:	d503201f 	nop
-[ ]*20001028:	00000000 	.inst	0x00000000 ; undefined
+[ ]*20001028:	00000000 	udf	#0
 [ ]*2000102c:	17fffff7 	b	20001008 <e843419\+0x10>
 	...
 
diff --git a/ld/testsuite/ld-aarch64/farcall-b-section.d b/ld/testsuite/ld-aarch64/farcall-b-section.d
index 7314eaf..40b1072 100644
--- a/ld/testsuite/ld-aarch64/farcall-b-section.d
+++ b/ld/testsuite/ld-aarch64/farcall-b-section.d
@@ -19,7 +19,7 @@ Disassembly of section .text:
     1018:	90040010 	adrp	x16, 8001000 <bar>
     101c:	91001210 	add	x16, x16, #0x4
     1020:	d61f0200 	br	x16
-    1024:	00000000 	.inst	0x00000000 ; undefined
+    1024:	00000000 	udf	#0
 
 .* <___veneer>:
     1028:	90040010 	adrp	x16, 8001000 <bar>
diff --git a/ld/testsuite/ld-aarch64/farcall-back.d b/ld/testsuite/ld-aarch64/farcall-back.d
index fcd0a29..20204ee 100644
--- a/ld/testsuite/ld-aarch64/farcall-back.d
+++ b/ld/testsuite/ld-aarch64/farcall-back.d
@@ -27,7 +27,7 @@ Disassembly of section .text:
     2028:	f07ffff0 	adrp	x16, 100001000 <bar1\+0x1000>
     202c:	91002210 	add	x16, x16, #0x8
     2030:	d61f0200 	br	x16
-    2034:	00000000 	.inst	0x00000000 ; undefined
+    2034:	00000000 	udf	#0
 
 0000000000002038 <__bar3_veneer>:
     2038:	58000090 	ldr	x16, 2048 <__bar3_veneer\+0x10>
diff --git a/ld/testsuite/ld-aarch64/farcall-bl-section.d b/ld/testsuite/ld-aarch64/farcall-bl-section.d
index 86b7a0b..b3ed36f 100644
--- a/ld/testsuite/ld-aarch64/farcall-bl-section.d
+++ b/ld/testsuite/ld-aarch64/farcall-bl-section.d
@@ -19,7 +19,7 @@ Disassembly of section .text:
     1018:	90040010 	adrp	x16, 8001000 <bar>
     101c:	91001210 	add	x16, x16, #0x4
     1020:	d61f0200 	br	x16
-    1024:	00000000 	.inst	0x00000000 ; undefined
+    1024:	00000000 	udf	#0
 
 .* <___veneer>:
     1028:	90040010 	adrp	x16, 8001000 <bar>
diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
index 54701ff..e917d61 100644
--- a/opcodes/aarch64-opc.c
+++ b/opcodes/aarch64-opc.c
@@ -251,6 +251,7 @@ const aarch64_field fields[] =
     { 10, 12 },	/* imm12: in ld/st unsigned imm or add/sub shifted inst.  */
     {  5, 14 },	/* imm14: in test bit and branch instructions.  */
     {  5, 16 },	/* imm16: in exception instructions.  */
+    {  0, 16 },	/* imm16_2: in udf instruction. */
     {  0, 26 },	/* imm26: in unconditional branch instructions.  */
     { 10,  6 },	/* imms: in bitfield and logical immediate instructions.  */
     { 16,  6 },	/* immr: in bitfield and logical immediate instructions.  */
@@ -3357,6 +3358,7 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc,
     case AARCH64_OPND_IMM0:
     case AARCH64_OPND_IMMR:
     case AARCH64_OPND_IMMS:
+    case AARCH64_OPND_UNDEFINED:
     case AARCH64_OPND_FBITS:
     case AARCH64_OPND_TME_UIMM16:
     case AARCH64_OPND_SIMM5:
diff --git a/opcodes/aarch64-opc.h b/opcodes/aarch64-opc.h
index 709d519..a197df6 100644
--- a/opcodes/aarch64-opc.h
+++ b/opcodes/aarch64-opc.h
@@ -78,6 +78,7 @@ enum aarch64_field_kind
   FLD_imm12,
   FLD_imm14,
   FLD_imm16,
+  FLD_imm16_2,
   FLD_imm26,
   FLD_imms,
   FLD_immr,
diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
index 46c0386..5ad7180 100644
--- a/opcodes/aarch64-tbl.h
+++ b/opcodes/aarch64-tbl.h
@@ -3354,6 +3354,7 @@ struct aarch64_opcode aarch64_opcode_table[] =
   CORE_INSN ("smc",   0xd4000003, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, 0),
   CORE_INSN ("brk",   0xd4200000, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, 0),
   CORE_INSN ("hlt",   0xd4400000, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, 0),
+  CORE_INSN ("udf",   0x00000000, 0xffff0000, exception, 0, OP1 (UNDEFINED), {}, 0),
   CORE_INSN ("dcps1", 0xd4a00001, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, F_OPD0_OPT | F_DEFAULT (0)),
   CORE_INSN ("dcps2", 0xd4a00002, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, F_OPD0_OPT | F_DEFAULT (0)),
   CORE_INSN ("dcps3", 0xd4a00003, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, F_OPD0_OPT | F_DEFAULT (0)),
@@ -5238,6 +5239,8 @@ struct aarch64_opcode aarch64_opcode_table[] =
       "the bit number to be tested")					\
     Y(IMMEDIATE, imm, "EXCEPTION", 0, F(FLD_imm16),			\
       "a 16-bit unsigned immediate")					\
+    Y(IMMEDIATE, imm, "UNDEFINED", 0, F(FLD_imm16_2),			\
+      "a 16-bit unsigned immediate")					\
     Y(IMMEDIATE, imm, "CCMP_IMM", 0, F(FLD_imm5),			\
       "a 5-bit unsigned immediate")					\
     Y(IMMEDIATE, imm, "SIMM5", OPD_F_SEXT, F(FLD_imm5),			\

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] AArch64: add GAS support for UDF instruction
  2020-04-28 10:42 [PATCH] AArch64: add GAS support for UDF instruction Alex Coplan
@ 2020-04-29  7:44 ` Tamar Christina
  2020-04-29 15:01   ` Alex Coplan
  0 siblings, 1 reply; 9+ messages in thread
From: Tamar Christina @ 2020-04-29  7:44 UTC (permalink / raw)
  To: Alex Coplan, binutils; +Cc: nd

Hi Alex,

I'm not a maintainer so can't approve your patch but please add a regression test to check something other than immediate 0.

Also your new field encoder isn't handing when the immediate is out of range correctly:

> printf "udf 0xEFFFC\n" | ./gas/as-new && ./binutils/objdump -d a.out | 
> tail -1

   0:   0000fffc        udf     #65532

Shows it's silently truncating the immediate rather than raising an out of range error.

PS: When submitting a patch also CC the maintainers for the area you're submitting for.

Thanks,
Tamar

> -----Original Message-----
> From: Binutils <binutils-bounces@sourceware.org> On Behalf Of Alex 
> Coplan
> Sent: Tuesday, April 28, 2020 11:42 AM
> To: binutils@sourceware.org
> Cc: nd <nd@arm.com>
> Subject: [PATCH] AArch64: add GAS support for UDF instruction
> 
> Hello,
> 
> The attached patch adds support to the AArch64 assembler for the UDF 
> instruction, see https://developer.arm.com/docs/ddi0596/a/a64-base-
> instructions-alphabetic-order/udf-permanently-undefined
> 
> Note that the files marked regenerated are not included in the patch 
> in order to keep the size of the patch down: these will need to be 
> regenerated before commit.
> 
> Testing:
>  - New regression test added: fails before and passes after the patch.
>  - Bootstrap and regression on aarch64-linux.
> 
> Ok for master? If so, I will need a maintainer to regenerate the 
> generated files and commit the patch as I don't have commit access.
> 
> Thanks,
> Alex
> 
> ---
> 
> binutils/ChangeLog:
> 
> 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> 
>         * testsuite/binutils-all/aarch64/in-order-all.d: Update to use new
>           disassembly.
>         * testsuite/binutils-all/aarch64/out-of-order-all.d: Likewise.
> 
> ld/ChangeLog:
> 
> 2020-04-28  Alex Coplan  <alex.coplan@arm.com>
> 
>         * testsuite/ld-aarch64/erratum843419_tls_ie.d: Use udf in disassembly.
>         * testsuite/ld-aarch64/farcall-b-section.d: Likewise.
>         * testsuite/ld-aarch64/farcall-back.d: Likewise.
>         * testsuite/ld-aarch64/farcall-bl-section.d: Likewise.
> 
> gas/ChangeLog:
> 
> 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> 
>         * config/tc-aarch64.c (fix_insn): Implement for 
> AARCH64_OPND_UNDEFINED.
>         * config/tc-aarch64.c (parse_operands): Likewise.
>         * testsuite/gas/aarch64/udf.s: New.
>         * testsuite/gas/aarch64/udf.d: New.
> 
> include/ChangeLog:
> 
> 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> 
>         * opcode/aarch64.h (enum aarch64_opnd): Add 
> AARCH64_OPND_UNDEFINED.
> 
> opcodes/ChangeLog:
> 
> 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> 
>         * aarch64-opc.h (enum aarch64_field_kind): Add FLD_imm16_2.
>         * aarch64-opc.c (fields): Add entry for FLD_imm16_2.
>         * aarch64-tbl.h (aarch64_opcode_table): Add udf instruction, entry for
>           FLD_imm16_2.
>         * aarch64-asm-2.c: Regenerated.
>         * aarch64-dis-2.c: Regenerated.
>         * aarch64-opc-2.c: Regenerated.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] AArch64: add GAS support for UDF instruction
  2020-04-29  7:44 ` Tamar Christina
@ 2020-04-29 15:01   ` Alex Coplan
  2020-04-29 15:09     ` Alex Coplan
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Coplan @ 2020-04-29 15:01 UTC (permalink / raw)
  To: Tamar Christina, binutils; +Cc: nd, Richard Earnshaw, Marcus Shawcroft

[-- Attachment #1: Type: text/plain, Size: 4864 bytes --]

Hi Tamar,

> -----Original Message-----
> From: Tamar Christina <Tamar.Christina@arm.com>
> Sent: 29 April 2020 08:44
> To: Alex Coplan <Alex.Coplan@arm.com>; binutils@sourceware.org
> Cc: nd <nd@arm.com>
> Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> 
> Hi Alex,
> 
> I'm not a maintainer so can't approve your patch but please add a
> regression test to check something other than immediate 0.

So my original patch introduced testsuite/gas/aarch64/udf.{s,d} which tests a
couple of different immediates. However, as your comment below reveals, it would
have been a good idea to test some error cases as well, so I've added
testsuite/gas/aarch64/udf-invalid.{s,l,d} to that end.

> 
> Also your new field encoder isn't handing when the immediate is out of
> range correctly:
> 
> > printf "udf 0xEFFFC\n" | ./gas/as-new && ./binutils/objdump -d a.out |
> > tail -1
> 
>    0:   0000fffc        udf     #65532
> 
> Shows it's silently truncating the immediate rather than raising an out
> of range error.

Good catch. The attached diff should fix this issue.

> 
> PS: When submitting a patch also CC the maintainers for the area you're
> submitting for.

Noted. Is the updated patch OK for master?

> 
> Thanks,
> Tamar

Thanks,
Alex

> -----Original Message-----
> From: Tamar Christina <Tamar.Christina@arm.com>
> Sent: 29 April 2020 08:44
> To: Alex Coplan <Alex.Coplan@arm.com>; binutils@sourceware.org
> Cc: nd <nd@arm.com>
> Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> 
> Hi Alex,
> 
> I'm not a maintainer so can't approve your patch but please add a
> regression test to check something other than immediate 0.
> 
> Also your new field encoder isn't handing when the immediate is out of
> range correctly:
> 
> > printf "udf 0xEFFFC\n" | ./gas/as-new && ./binutils/objdump -d a.out |
> > tail -1
> 
>    0:   0000fffc        udf     #65532
> 
> Shows it's silently truncating the immediate rather than raising an out
> of range error.
> 
> PS: When submitting a patch also CC the maintainers for the area you're
> submitting for.
> 
> Thanks,
> Tamar
> 
> > -----Original Message-----
> > From: Binutils <binutils-bounces@sourceware.org> On Behalf Of Alex
> > Coplan
> > Sent: Tuesday, April 28, 2020 11:42 AM
> > To: binutils@sourceware.org
> > Cc: nd <nd@arm.com>
> > Subject: [PATCH] AArch64: add GAS support for UDF instruction
> >
> > Hello,
> >
> > The attached patch adds support to the AArch64 assembler for the UDF
> > instruction, see https://developer.arm.com/docs/ddi0596/a/a64-base-
> > instructions-alphabetic-order/udf-permanently-undefined
> >
> > Note that the files marked regenerated are not included in the patch
> > in order to keep the size of the patch down: these will need to be
> > regenerated before commit.
> >
> > Testing:
> >  - New regression test added: fails before and passes after the patch.
> >  - Bootstrap and regression on aarch64-linux.
> >
> > Ok for master? If so, I will need a maintainer to regenerate the
> > generated files and commit the patch as I don't have commit access.
> >
> > Thanks,
> > Alex
> >
> > ---
> >
> > binutils/ChangeLog:
> >
> > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> >
> >         * testsuite/binutils-all/aarch64/in-order-all.d: Update to use
> new
> >           disassembly.
> >         * testsuite/binutils-all/aarch64/out-of-order-all.d: Likewise.
> >
> > ld/ChangeLog:
> >
> > 2020-04-28  Alex Coplan  <alex.coplan@arm.com>
> >
> >         * testsuite/ld-aarch64/erratum843419_tls_ie.d: Use udf in
> disassembly.
> >         * testsuite/ld-aarch64/farcall-b-section.d: Likewise.
> >         * testsuite/ld-aarch64/farcall-back.d: Likewise.
> >         * testsuite/ld-aarch64/farcall-bl-section.d: Likewise.
> >
> > gas/ChangeLog:
> >
> > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> >
> >         * config/tc-aarch64.c (fix_insn): Implement for
> > AARCH64_OPND_UNDEFINED.
> >         * config/tc-aarch64.c (parse_operands): Likewise.
> >         * testsuite/gas/aarch64/udf.s: New.
> >         * testsuite/gas/aarch64/udf.d: New.
> >
> > include/ChangeLog:
> >
> > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> >
> >         * opcode/aarch64.h (enum aarch64_opnd): Add
> > AARCH64_OPND_UNDEFINED.
> >
> > opcodes/ChangeLog:
> >
> > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> >
> >         * aarch64-opc.h (enum aarch64_field_kind): Add FLD_imm16_2.
> >         * aarch64-opc.c (fields): Add entry for FLD_imm16_2.
> >         * aarch64-tbl.h (aarch64_opcode_table): Add udf instruction,
> entry for
> >           FLD_imm16_2.
> >         * aarch64-asm-2.c: Regenerated.
> >         * aarch64-dis-2.c: Regenerated.
> >         * aarch64-opc-2.c: Regenerated.

[-- Attachment #2: patch_v2.txt --]
[-- Type: text/plain, Size: 10461 bytes --]

diff --git a/binutils/testsuite/binutils-all/aarch64/in-order-all.d b/binutils/testsuite/binutils-all/aarch64/in-order-all.d
index a484ca7d17..a6daea8145 100644
--- a/binutils/testsuite/binutils-all/aarch64/in-order-all.d
+++ b/binutils/testsuite/binutils-all/aarch64/in-order-all.d
@@ -10,7 +10,7 @@ Disassembly of section \.func1:
 
 .+ <v1>:
 [^:]+:	8b010000 	add	x0, x0, x1
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section \.func2:
 
@@ -25,12 +25,12 @@ Disassembly of section \.func3:
 [^:]+:	8b010000 	add	x0, x0, x1
 [^:]+:	8b010000 	add	x0, x0, x1
 [^:]+:	8b010000 	add	x0, x0, x1
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section \.rodata:
 
 .+ <\.rodata>:
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section .global:
 
diff --git a/binutils/testsuite/binutils-all/aarch64/out-of-order-all.d b/binutils/testsuite/binutils-all/aarch64/out-of-order-all.d
index d3aa79e482..955d190465 100644
--- a/binutils/testsuite/binutils-all/aarch64/out-of-order-all.d
+++ b/binutils/testsuite/binutils-all/aarch64/out-of-order-all.d
@@ -20,7 +20,7 @@ Disassembly of section \.func1:
 
 .+ <v1>:
 [^:]+:	8b010000 	add	x0, x0, x1
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section \.func3:
 
@@ -30,9 +30,9 @@ Disassembly of section \.func3:
 [^:]+:	8b010000 	add	x0, x0, x1
 [^:]+:	8b010000 	add	x0, x0, x1
 [^:]+:	8b010000 	add	x0, x0, x1
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
 
 Disassembly of section \.rodata:
 
 .+ <\.rodata>:
-[^:]+:	00000000 	\.inst	0x00000000 ; undefined
+[^:]+:	00000000 	udf	#0
diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 69ccc59e87..da786ba5e9 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -6149,6 +6149,7 @@ parse_operands (char *str, const aarch64_opcode *opcode)
 	  break;
 
 	case AARCH64_OPND_EXCEPTION:
+	case AARCH64_OPND_UNDEFINED:
 	  po_misc_or_fail (parse_immediate_expression (&str, &inst.reloc.exp,
 						       imm_reg_type));
 	  assign_imm_if_const_or_fixup_later (&inst.reloc, info,
@@ -7745,11 +7746,12 @@ fix_insn (fixS *fixP, uint32_t flags, offsetT value)
   switch (opnd)
     {
     case AARCH64_OPND_EXCEPTION:
+    case AARCH64_OPND_UNDEFINED:
       if (unsigned_overflow (value, 16))
 	as_bad_where (fixP->fx_file, fixP->fx_line,
 		      _("immediate out of range"));
       insn = get_aarch64_insn (buf);
-      insn |= encode_svc_imm (value);
+      insn |= (opnd == AARCH64_OPND_EXCEPTION) ? encode_svc_imm (value) : value;
       put_aarch64_insn (buf, insn);
       break;
 
diff --git a/gas/testsuite/gas/aarch64/udf-invalid.d b/gas/testsuite/gas/aarch64/udf-invalid.d
new file mode 100644
index 0000000000..bdb768d8a3
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/udf-invalid.d
@@ -0,0 +1,3 @@
+#name: invalid udf instructions
+#source: udf-invalid.s
+#error_output: udf-invalid.l
diff --git a/gas/testsuite/gas/aarch64/udf-invalid.l b/gas/testsuite/gas/aarch64/udf-invalid.l
new file mode 100644
index 0000000000..71a0791151
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/udf-invalid.l
@@ -0,0 +1,4 @@
+[^:]*: Assembler messages:
+.*: Error: immediate value out of range 0 to 65535 at operand 1 -- `udf #65536'
+.*: Error: immediate value out of range 0 to 65535 at operand 1 -- `udf 0xeffff'
+.*: Error: immediate value out of range 0 to 65535 at operand 1 -- `udf -1'
diff --git a/gas/testsuite/gas/aarch64/udf-invalid.s b/gas/testsuite/gas/aarch64/udf-invalid.s
new file mode 100644
index 0000000000..937126fcf5
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/udf-invalid.s
@@ -0,0 +1,6 @@
+// Instructions in this file are invalid.
+// See udf.s for valid instructions.
+.text
+udf #65536
+udf 0xeffff
+udf -1
diff --git a/gas/testsuite/gas/aarch64/udf.d b/gas/testsuite/gas/aarch64/udf.d
new file mode 100644
index 0000000000..ae6432b36f
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/udf.d
@@ -0,0 +1,9 @@
+#objdump: -dr
+
+.*:     file format .*
+
+Disassembly of section \.text:
+
+0+ <.*>:
+.*:	0000002a 	udf	#42
+.*:	0000ffff 	udf	#65535
diff --git a/gas/testsuite/gas/aarch64/udf.s b/gas/testsuite/gas/aarch64/udf.s
new file mode 100644
index 0000000000..fec30a3105
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/udf.s
@@ -0,0 +1,5 @@
+// Test file for AArch64 udf.
+
+.text
+udf #42
+udf #65535
diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h
index c15295240e..817ca1e674 100644
--- a/include/opcode/aarch64.h
+++ b/include/opcode/aarch64.h
@@ -272,6 +272,7 @@ enum aarch64_opnd
   AARCH64_OPND_UIMM10,	/* Unsigned 10-bit immediate in addg/subg.  */
   AARCH64_OPND_BIT_NUM,	/* Immediate.  */
   AARCH64_OPND_EXCEPTION,/* imm16 operand in exception instructions.  */
+  AARCH64_OPND_UNDEFINED,/* imm16 operand in undefined instruction. */
   AARCH64_OPND_CCMP_IMM,/* Immediate in conditional compare instructions.  */
   AARCH64_OPND_SIMM5,	/* 5-bit signed immediate in the imm5 field.  */
   AARCH64_OPND_NZCV,	/* Flag bit specifier giving an alternative value for
diff --git a/ld/testsuite/ld-aarch64/erratum843419_tls_ie.d b/ld/testsuite/ld-aarch64/erratum843419_tls_ie.d
index eba5a20217..4d2b111644 100644
--- a/ld/testsuite/ld-aarch64/erratum843419_tls_ie.d
+++ b/ld/testsuite/ld-aarch64/erratum843419_tls_ie.d
@@ -23,10 +23,10 @@ Disassembly of section .e843419:
 [ ]*20001010:	0b0700e0 	add	w0, w7, w7
 [ ]*20001014:	910043ff 	add	sp, sp, #0x10
 [ ]*20001018:	d65f03c0 	ret
-[ ]*2000101c:	00000000 	.inst	0x00000000 ; undefined
+[ ]*2000101c:	00000000 	udf	#0
 [ ]*20001020:	14000400 	b	20002020 <e843419\+0x1028>
 [ ]*20001024:	d503201f 	nop
-[ ]*20001028:	00000000 	.inst	0x00000000 ; undefined
+[ ]*20001028:	00000000 	udf	#0
 [ ]*2000102c:	17fffff7 	b	20001008 <e843419\+0x10>
 	...
 
diff --git a/ld/testsuite/ld-aarch64/farcall-b-section.d b/ld/testsuite/ld-aarch64/farcall-b-section.d
index 7314eafa61..40b1072095 100644
--- a/ld/testsuite/ld-aarch64/farcall-b-section.d
+++ b/ld/testsuite/ld-aarch64/farcall-b-section.d
@@ -19,7 +19,7 @@ Disassembly of section .text:
     1018:	90040010 	adrp	x16, 8001000 <bar>
     101c:	91001210 	add	x16, x16, #0x4
     1020:	d61f0200 	br	x16
-    1024:	00000000 	.inst	0x00000000 ; undefined
+    1024:	00000000 	udf	#0
 
 .* <___veneer>:
     1028:	90040010 	adrp	x16, 8001000 <bar>
diff --git a/ld/testsuite/ld-aarch64/farcall-back.d b/ld/testsuite/ld-aarch64/farcall-back.d
index fcd0a296ff..20204eef5e 100644
--- a/ld/testsuite/ld-aarch64/farcall-back.d
+++ b/ld/testsuite/ld-aarch64/farcall-back.d
@@ -27,7 +27,7 @@ Disassembly of section .text:
     2028:	f07ffff0 	adrp	x16, 100001000 <bar1\+0x1000>
     202c:	91002210 	add	x16, x16, #0x8
     2030:	d61f0200 	br	x16
-    2034:	00000000 	.inst	0x00000000 ; undefined
+    2034:	00000000 	udf	#0
 
 0000000000002038 <__bar3_veneer>:
     2038:	58000090 	ldr	x16, 2048 <__bar3_veneer\+0x10>
diff --git a/ld/testsuite/ld-aarch64/farcall-bl-section.d b/ld/testsuite/ld-aarch64/farcall-bl-section.d
index 86b7a0bb8c..b3ed36fbb9 100644
--- a/ld/testsuite/ld-aarch64/farcall-bl-section.d
+++ b/ld/testsuite/ld-aarch64/farcall-bl-section.d
@@ -19,7 +19,7 @@ Disassembly of section .text:
     1018:	90040010 	adrp	x16, 8001000 <bar>
     101c:	91001210 	add	x16, x16, #0x4
     1020:	d61f0200 	br	x16
-    1024:	00000000 	.inst	0x00000000 ; undefined
+    1024:	00000000 	udf	#0
 
 .* <___veneer>:
     1028:	90040010 	adrp	x16, 8001000 <bar>
diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
index 54701ffa1b..faa0503dcf 100644
--- a/opcodes/aarch64-opc.c
+++ b/opcodes/aarch64-opc.c
@@ -251,6 +251,7 @@ const aarch64_field fields[] =
     { 10, 12 },	/* imm12: in ld/st unsigned imm or add/sub shifted inst.  */
     {  5, 14 },	/* imm14: in test bit and branch instructions.  */
     {  5, 16 },	/* imm16: in exception instructions.  */
+    {  0, 16 },	/* imm16_2: in udf instruction. */
     {  0, 26 },	/* imm26: in unconditional branch instructions.  */
     { 10,  6 },	/* imms: in bitfield and logical immediate instructions.  */
     { 16,  6 },	/* immr: in bitfield and logical immediate instructions.  */
@@ -2145,6 +2146,7 @@ operand_general_constraint_met_p (const aarch64_opnd_info *opnds, int idx,
 	case AARCH64_OPND_NZCV:
 	case AARCH64_OPND_CCMP_IMM:
 	case AARCH64_OPND_EXCEPTION:
+	case AARCH64_OPND_UNDEFINED:
 	case AARCH64_OPND_TME_UIMM16:
 	case AARCH64_OPND_UIMM4:
 	case AARCH64_OPND_UIMM4_ADDG:
@@ -3357,6 +3359,7 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc,
     case AARCH64_OPND_IMM0:
     case AARCH64_OPND_IMMR:
     case AARCH64_OPND_IMMS:
+    case AARCH64_OPND_UNDEFINED:
     case AARCH64_OPND_FBITS:
     case AARCH64_OPND_TME_UIMM16:
     case AARCH64_OPND_SIMM5:
diff --git a/opcodes/aarch64-opc.h b/opcodes/aarch64-opc.h
index 709d5192b0..a197df69d8 100644
--- a/opcodes/aarch64-opc.h
+++ b/opcodes/aarch64-opc.h
@@ -78,6 +78,7 @@ enum aarch64_field_kind
   FLD_imm12,
   FLD_imm14,
   FLD_imm16,
+  FLD_imm16_2,
   FLD_imm26,
   FLD_imms,
   FLD_immr,
diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
index 46c0386eed..5ad718014e 100644
--- a/opcodes/aarch64-tbl.h
+++ b/opcodes/aarch64-tbl.h
@@ -3354,6 +3354,7 @@ struct aarch64_opcode aarch64_opcode_table[] =
   CORE_INSN ("smc",   0xd4000003, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, 0),
   CORE_INSN ("brk",   0xd4200000, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, 0),
   CORE_INSN ("hlt",   0xd4400000, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, 0),
+  CORE_INSN ("udf",   0x00000000, 0xffff0000, exception, 0, OP1 (UNDEFINED), {}, 0),
   CORE_INSN ("dcps1", 0xd4a00001, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, F_OPD0_OPT | F_DEFAULT (0)),
   CORE_INSN ("dcps2", 0xd4a00002, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, F_OPD0_OPT | F_DEFAULT (0)),
   CORE_INSN ("dcps3", 0xd4a00003, 0xffe0001f, exception, 0, OP1 (EXCEPTION), {}, F_OPD0_OPT | F_DEFAULT (0)),
@@ -5238,6 +5239,8 @@ struct aarch64_opcode aarch64_opcode_table[] =
       "the bit number to be tested")					\
     Y(IMMEDIATE, imm, "EXCEPTION", 0, F(FLD_imm16),			\
       "a 16-bit unsigned immediate")					\
+    Y(IMMEDIATE, imm, "UNDEFINED", 0, F(FLD_imm16_2),			\
+      "a 16-bit unsigned immediate")					\
     Y(IMMEDIATE, imm, "CCMP_IMM", 0, F(FLD_imm5),			\
       "a 5-bit unsigned immediate")					\
     Y(IMMEDIATE, imm, "SIMM5", OPD_F_SEXT, F(FLD_imm5),			\

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] AArch64: add GAS support for UDF instruction
  2020-04-29 15:01   ` Alex Coplan
@ 2020-04-29 15:09     ` Alex Coplan
  2020-04-29 19:29       ` Tamar Christina
  0 siblings, 1 reply; 9+ messages in thread
From: Alex Coplan @ 2020-04-29 15:09 UTC (permalink / raw)
  To: Alex Coplan, Tamar Christina, binutils
  Cc: Richard Earnshaw, nd, Marcus Shawcroft

> -----Original Message-----
> From: Binutils <binutils-bounces@sourceware.org> On Behalf Of Alex Coplan
> Sent: 29 April 2020 16:02
> To: Tamar Christina <Tamar.Christina@arm.com>; binutils@sourceware.org
> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; nd <nd@arm.com>; Marcus
> Shawcroft <Marcus.Shawcroft@arm.com>
> Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> 
> Hi Tamar,
> 
> > -----Original Message-----
> > From: Tamar Christina <Tamar.Christina@arm.com>
> > Sent: 29 April 2020 08:44
> > To: Alex Coplan <Alex.Coplan@arm.com>; binutils@sourceware.org
> > Cc: nd <nd@arm.com>
> > Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> >
> > Hi Alex,
> >
> > I'm not a maintainer so can't approve your patch but please add a
> > regression test to check something other than immediate 0.
> 
> So my original patch introduced testsuite/gas/aarch64/udf.{s,d} which
> tests a
> couple of different immediates. However, as your comment below reveals,
> it would
> have been a good idea to test some error cases as well, so I've added
> testsuite/gas/aarch64/udf-invalid.{s,l,d} to that end.
> 
> >
> > Also your new field encoder isn't handing when the immediate is out of
> > range correctly:
> >
> > > printf "udf 0xEFFFC\n" | ./gas/as-new && ./binutils/objdump -d a.out
> |
> > > tail -1
> >
> >    0:   0000fffc        udf     #65532
> >
> > Shows it's silently truncating the immediate rather than raising an out
> > of range error.
> 
> Good catch. The attached diff should fix this issue.
> 
> >
> > PS: When submitting a patch also CC the maintainers for the area you're
> > submitting for.
> 
> Noted. Is the updated patch OK for master?
> 
> >
> > Thanks,
> > Tamar
> 
> Thanks,
> Alex

I forgot to include the updated ChangeLog. Here it is:

binutils/ChangeLog:

2020-04-27  Alex Coplan  <alex.coplan@arm.com>

        * testsuite/binutils-all/aarch64/in-order-all.d: Update to use new
          disassembly.
        * testsuite/binutils-all/aarch64/out-of-order-all.d: Likewise.

ld/ChangeLog:

2020-04-28  Alex Coplan  <alex.coplan@arm.com>

        * testsuite/ld-aarch64/erratum843419_tls_ie.d: Use udf in disassembly.
        * testsuite/ld-aarch64/farcall-b-section.d: Likewise.
        * testsuite/ld-aarch64/farcall-back.d: Likewise.
        * testsuite/ld-aarch64/farcall-bl-section.d: Likewise.

gas/ChangeLog:

2020-04-27  Alex Coplan  <alex.coplan@arm.com>

        * config/tc-aarch64.c (fix_insn): Implement for AARCH64_OPND_UNDEFINED.
          (parse_operands): Implement for AARCH64_OPND_UNDEFINED.
        * testsuite/gas/aarch64/udf.s: New.
        * testsuite/gas/aarch64/udf.d: New.
        * testsuite/gas/aarch64/udf-invalid.s: New.
        * testsuite/gas/aarch64/udf-invalid.l: New.
        * testsuite/gas/aarch64/udf-invalid.d: New.

include/ChangeLog:

2020-04-27  Alex Coplan  <alex.coplan@arm.com>

        * opcode/aarch64.h (enum aarch64_opnd): Add AARCH64_OPND_UNDEFINED.

opcodes/ChangeLog:

2020-04-27  Alex Coplan  <alex.coplan@arm.com>

        * aarch64-opc.h (enum aarch64_field_kind): Add FLD_imm16_2.
        * aarch64-opc.c (fields): Add entry for FLD_imm16_2.
          (operand_general_constraint_met_p): validate AARCH64_OPND_UNDEFINED.
        * aarch64-tbl.h (aarch64_opcode_table): Add udf instruction, entry for
          FLD_imm16_2.
        * aarch64-asm-2.c: Regenerated.
        * aarch64-dis-2.c: Regenerated.
        * aarch64-opc-2.c: Regenerated.

> 
> > -----Original Message-----
> > From: Tamar Christina <Tamar.Christina@arm.com>
> > Sent: 29 April 2020 08:44
> > To: Alex Coplan <Alex.Coplan@arm.com>; binutils@sourceware.org
> > Cc: nd <nd@arm.com>
> > Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> >
> > Hi Alex,
> >
> > I'm not a maintainer so can't approve your patch but please add a
> > regression test to check something other than immediate 0.
> >
> > Also your new field encoder isn't handing when the immediate is out of
> > range correctly:
> >
> > > printf "udf 0xEFFFC\n" | ./gas/as-new && ./binutils/objdump -d a.out
> |
> > > tail -1
> >
> >    0:   0000fffc        udf     #65532
> >
> > Shows it's silently truncating the immediate rather than raising an out
> > of range error.
> >
> > PS: When submitting a patch also CC the maintainers for the area you're
> > submitting for.
> >
> > Thanks,
> > Tamar
> >
> > > -----Original Message-----
> > > From: Binutils <binutils-bounces@sourceware.org> On Behalf Of Alex
> > > Coplan
> > > Sent: Tuesday, April 28, 2020 11:42 AM
> > > To: binutils@sourceware.org
> > > Cc: nd <nd@arm.com>
> > > Subject: [PATCH] AArch64: add GAS support for UDF instruction
> > >
> > > Hello,
> > >
> > > The attached patch adds support to the AArch64 assembler for the UDF
> > > instruction, see https://developer.arm.com/docs/ddi0596/a/a64-base-
> > > instructions-alphabetic-order/udf-permanently-undefined
> > >
> > > Note that the files marked regenerated are not included in the patch
> > > in order to keep the size of the patch down: these will need to be
> > > regenerated before commit.
> > >
> > > Testing:
> > >  - New regression test added: fails before and passes after the
> patch.
> > >  - Bootstrap and regression on aarch64-linux.
> > >
> > > Ok for master? If so, I will need a maintainer to regenerate the
> > > generated files and commit the patch as I don't have commit access.
> > >
> > > Thanks,
> > > Alex
> > >
> > > ---
> > >
> > > binutils/ChangeLog:
> > >
> > > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> > >
> > >         * testsuite/binutils-all/aarch64/in-order-all.d: Update to
> use
> > new
> > >           disassembly.
> > >         * testsuite/binutils-all/aarch64/out-of-order-all.d:
> Likewise.
> > >
> > > ld/ChangeLog:
> > >
> > > 2020-04-28  Alex Coplan  <alex.coplan@arm.com>
> > >
> > >         * testsuite/ld-aarch64/erratum843419_tls_ie.d: Use udf in
> > disassembly.
> > >         * testsuite/ld-aarch64/farcall-b-section.d: Likewise.
> > >         * testsuite/ld-aarch64/farcall-back.d: Likewise.
> > >         * testsuite/ld-aarch64/farcall-bl-section.d: Likewise.
> > >
> > > gas/ChangeLog:
> > >
> > > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> > >
> > >         * config/tc-aarch64.c (fix_insn): Implement for
> > > AARCH64_OPND_UNDEFINED.
> > >         * config/tc-aarch64.c (parse_operands): Likewise.
> > >         * testsuite/gas/aarch64/udf.s: New.
> > >         * testsuite/gas/aarch64/udf.d: New.
> > >
> > > include/ChangeLog:
> > >
> > > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> > >
> > >         * opcode/aarch64.h (enum aarch64_opnd): Add
> > > AARCH64_OPND_UNDEFINED.
> > >
> > > opcodes/ChangeLog:
> > >
> > > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> > >
> > >         * aarch64-opc.h (enum aarch64_field_kind): Add FLD_imm16_2.
> > >         * aarch64-opc.c (fields): Add entry for FLD_imm16_2.
> > >         * aarch64-tbl.h (aarch64_opcode_table): Add udf instruction,
> > entry for
> > >           FLD_imm16_2.
> > >         * aarch64-asm-2.c: Regenerated.
> > >         * aarch64-dis-2.c: Regenerated.
> > >         * aarch64-opc-2.c: Regenerated.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] AArch64: add GAS support for UDF instruction
  2020-04-29 15:09     ` Alex Coplan
@ 2020-04-29 19:29       ` Tamar Christina
  2020-04-30  9:25         ` Nick Clifton
  0 siblings, 1 reply; 9+ messages in thread
From: Tamar Christina @ 2020-04-29 19:29 UTC (permalink / raw)
  To: Alex Coplan, binutils; +Cc: Richard Earnshaw, nd, Marcus Shawcroft

Hi Alex,

Thanks for fixing it, this looks good to me but you still need a maintainer to approve.

Thanks,
Tamar


> -----Original Message-----
> From: Alex Coplan <Alex.Coplan@arm.com>
> Sent: Wednesday, April 29, 2020 4:09 PM
> To: Alex Coplan <Alex.Coplan@arm.com>; Tamar Christina
> <Tamar.Christina@arm.com>; binutils@sourceware.org
> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;
> Marcus Shawcroft <Marcus.Shawcroft@arm.com>
> Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> 
> > -----Original Message-----
> > From: Binutils <binutils-bounces@sourceware.org> On Behalf Of Alex
> > Coplan
> > Sent: 29 April 2020 16:02
> > To: Tamar Christina <Tamar.Christina@arm.com>; binutils@sourceware.org
> > Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;
> > Marcus Shawcroft <Marcus.Shawcroft@arm.com>
> > Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> >
> > Hi Tamar,
> >
> > > -----Original Message-----
> > > From: Tamar Christina <Tamar.Christina@arm.com>
> > > Sent: 29 April 2020 08:44
> > > To: Alex Coplan <Alex.Coplan@arm.com>; binutils@sourceware.org
> > > Cc: nd <nd@arm.com>
> > > Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> > >
> > > Hi Alex,
> > >
> > > I'm not a maintainer so can't approve your patch but please add a
> > > regression test to check something other than immediate 0.
> >
> > So my original patch introduced testsuite/gas/aarch64/udf.{s,d} which
> > tests a couple of different immediates. However, as your comment below
> > reveals, it would have been a good idea to test some error cases as
> > well, so I've added testsuite/gas/aarch64/udf-invalid.{s,l,d} to that
> > end.
> >
> > >
> > > Also your new field encoder isn't handing when the immediate is out
> > > of range correctly:
> > >
> > > > printf "udf 0xEFFFC\n" | ./gas/as-new && ./binutils/objdump -d
> > > > a.out
> > |
> > > > tail -1
> > >
> > >    0:   0000fffc        udf     #65532
> > >
> > > Shows it's silently truncating the immediate rather than raising an
> > > out of range error.
> >
> > Good catch. The attached diff should fix this issue.
> >
> > >
> > > PS: When submitting a patch also CC the maintainers for the area
> > > you're submitting for.
> >
> > Noted. Is the updated patch OK for master?
> >
> > >
> > > Thanks,
> > > Tamar
> >
> > Thanks,
> > Alex
> 
> I forgot to include the updated ChangeLog. Here it is:
> 
> binutils/ChangeLog:
> 
> 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> 
>         * testsuite/binutils-all/aarch64/in-order-all.d: Update to use new
>           disassembly.
>         * testsuite/binutils-all/aarch64/out-of-order-all.d: Likewise.
> 
> ld/ChangeLog:
> 
> 2020-04-28  Alex Coplan  <alex.coplan@arm.com>
> 
>         * testsuite/ld-aarch64/erratum843419_tls_ie.d: Use udf in disassembly.
>         * testsuite/ld-aarch64/farcall-b-section.d: Likewise.
>         * testsuite/ld-aarch64/farcall-back.d: Likewise.
>         * testsuite/ld-aarch64/farcall-bl-section.d: Likewise.
> 
> gas/ChangeLog:
> 
> 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> 
>         * config/tc-aarch64.c (fix_insn): Implement for
> AARCH64_OPND_UNDEFINED.
>           (parse_operands): Implement for AARCH64_OPND_UNDEFINED.
>         * testsuite/gas/aarch64/udf.s: New.
>         * testsuite/gas/aarch64/udf.d: New.
>         * testsuite/gas/aarch64/udf-invalid.s: New.
>         * testsuite/gas/aarch64/udf-invalid.l: New.
>         * testsuite/gas/aarch64/udf-invalid.d: New.
> 
> include/ChangeLog:
> 
> 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> 
>         * opcode/aarch64.h (enum aarch64_opnd): Add
> AARCH64_OPND_UNDEFINED.
> 
> opcodes/ChangeLog:
> 
> 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> 
>         * aarch64-opc.h (enum aarch64_field_kind): Add FLD_imm16_2.
>         * aarch64-opc.c (fields): Add entry for FLD_imm16_2.
>           (operand_general_constraint_met_p): validate
> AARCH64_OPND_UNDEFINED.
>         * aarch64-tbl.h (aarch64_opcode_table): Add udf instruction, entry for
>           FLD_imm16_2.
>         * aarch64-asm-2.c: Regenerated.
>         * aarch64-dis-2.c: Regenerated.
>         * aarch64-opc-2.c: Regenerated.
> 
> >
> > > -----Original Message-----
> > > From: Tamar Christina <Tamar.Christina@arm.com>
> > > Sent: 29 April 2020 08:44
> > > To: Alex Coplan <Alex.Coplan@arm.com>; binutils@sourceware.org
> > > Cc: nd <nd@arm.com>
> > > Subject: RE: [PATCH] AArch64: add GAS support for UDF instruction
> > >
> > > Hi Alex,
> > >
> > > I'm not a maintainer so can't approve your patch but please add a
> > > regression test to check something other than immediate 0.
> > >
> > > Also your new field encoder isn't handing when the immediate is out
> > > of range correctly:
> > >
> > > > printf "udf 0xEFFFC\n" | ./gas/as-new && ./binutils/objdump -d
> > > > a.out
> > |
> > > > tail -1
> > >
> > >    0:   0000fffc        udf     #65532
> > >
> > > Shows it's silently truncating the immediate rather than raising an
> > > out of range error.
> > >
> > > PS: When submitting a patch also CC the maintainers for the area
> > > you're submitting for.
> > >
> > > Thanks,
> > > Tamar
> > >
> > > > -----Original Message-----
> > > > From: Binutils <binutils-bounces@sourceware.org> On Behalf Of Alex
> > > > Coplan
> > > > Sent: Tuesday, April 28, 2020 11:42 AM
> > > > To: binutils@sourceware.org
> > > > Cc: nd <nd@arm.com>
> > > > Subject: [PATCH] AArch64: add GAS support for UDF instruction
> > > >
> > > > Hello,
> > > >
> > > > The attached patch adds support to the AArch64 assembler for the
> > > > UDF instruction, see
> > > > https://developer.arm.com/docs/ddi0596/a/a64-base-
> > > > instructions-alphabetic-order/udf-permanently-undefined
> > > >
> > > > Note that the files marked regenerated are not included in the
> > > > patch in order to keep the size of the patch down: these will need
> > > > to be regenerated before commit.
> > > >
> > > > Testing:
> > > >  - New regression test added: fails before and passes after the
> > patch.
> > > >  - Bootstrap and regression on aarch64-linux.
> > > >
> > > > Ok for master? If so, I will need a maintainer to regenerate the
> > > > generated files and commit the patch as I don't have commit access.
> > > >
> > > > Thanks,
> > > > Alex
> > > >
> > > > ---
> > > >
> > > > binutils/ChangeLog:
> > > >
> > > > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> > > >
> > > >         * testsuite/binutils-all/aarch64/in-order-all.d: Update to
> > use
> > > new
> > > >           disassembly.
> > > >         * testsuite/binutils-all/aarch64/out-of-order-all.d:
> > Likewise.
> > > >
> > > > ld/ChangeLog:
> > > >
> > > > 2020-04-28  Alex Coplan  <alex.coplan@arm.com>
> > > >
> > > >         * testsuite/ld-aarch64/erratum843419_tls_ie.d: Use udf in
> > > disassembly.
> > > >         * testsuite/ld-aarch64/farcall-b-section.d: Likewise.
> > > >         * testsuite/ld-aarch64/farcall-back.d: Likewise.
> > > >         * testsuite/ld-aarch64/farcall-bl-section.d: Likewise.
> > > >
> > > > gas/ChangeLog:
> > > >
> > > > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> > > >
> > > >         * config/tc-aarch64.c (fix_insn): Implement for
> > > > AARCH64_OPND_UNDEFINED.
> > > >         * config/tc-aarch64.c (parse_operands): Likewise.
> > > >         * testsuite/gas/aarch64/udf.s: New.
> > > >         * testsuite/gas/aarch64/udf.d: New.
> > > >
> > > > include/ChangeLog:
> > > >
> > > > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> > > >
> > > >         * opcode/aarch64.h (enum aarch64_opnd): Add
> > > > AARCH64_OPND_UNDEFINED.
> > > >
> > > > opcodes/ChangeLog:
> > > >
> > > > 2020-04-27  Alex Coplan  <alex.coplan@arm.com>
> > > >
> > > >         * aarch64-opc.h (enum aarch64_field_kind): Add FLD_imm16_2.
> > > >         * aarch64-opc.c (fields): Add entry for FLD_imm16_2.
> > > >         * aarch64-tbl.h (aarch64_opcode_table): Add udf
> > > > instruction,
> > > entry for
> > > >           FLD_imm16_2.
> > > >         * aarch64-asm-2.c: Regenerated.
> > > >         * aarch64-dis-2.c: Regenerated.
> > > >         * aarch64-opc-2.c: Regenerated.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] AArch64: add GAS support for UDF instruction
  2020-04-29 19:29       ` Tamar Christina
@ 2020-04-30  9:25         ` Nick Clifton
  2020-04-30  9:30           ` Tamar Christina
  0 siblings, 1 reply; 9+ messages in thread
From: Nick Clifton @ 2020-04-30  9:25 UTC (permalink / raw)
  To: Tamar Christina, Alex Coplan
  Cc: binutils, Richard Earnshaw, nd, Marcus Shawcroft

Hi Tamar, Hi Alex, 
> Thanks for fixing it, this looks good to me but you still need a maintainer to approve.
 
I encountered lots of new testsuite failures when I tested this patch.
At least I think that I tested it - I used "patch_v2.txt".  Was this the
correct file ?  If not then please can you point me at the correct patch ?

The failures were mostly connected with the disassembler not disassembling
correctly.  eg:

  regexp "^   0:	10000001 	adr	x1, 0 <bar>$"
  line   "   0:	10000001 	.inst	0x10000001 ; undefined"
  FAIL: gas/aarch64/adr_1

(This was from a toolchain configured as aarch64-linux-gnu).  But there were
also some internal errors as well. eg:

  failed with: </gas/testsuite/gas/aarch64/advsimd-armv8_3.s: Assembler messages:
  gas/testsuite/gas/aarch64/advsimd-armv8_3.s:60: Internal error (Segmentation fault).
  Please report this bug.>, no expected output
  FAIL: gas/aarch64/advsimd-armv8_3

Cheers
  Nick


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] AArch64: add GAS support for UDF instruction
  2020-04-30  9:25         ` Nick Clifton
@ 2020-04-30  9:30           ` Tamar Christina
  2020-04-30 11:50             ` Nick Clifton
  0 siblings, 1 reply; 9+ messages in thread
From: Tamar Christina @ 2020-04-30  9:30 UTC (permalink / raw)
  To: nickc, Alex Coplan; +Cc: binutils, Richard Earnshaw, nd, Marcus Shawcroft

Hi Nick,

Just to double check, did you remember to regenerate the -2.c files?

Alex's patch omitted them and your error looks consistent with it using an out of sync definition.

Cheers,
Tamar

> -----Original Message-----
> From: Nick Clifton <nickc@redhat.com>
> Sent: Thursday, April 30, 2020 10:26 AM
> To: Tamar Christina <Tamar.Christina@arm.com>; Alex Coplan
> <Alex.Coplan@arm.com>
> Cc: binutils@sourceware.org; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; nd <nd@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>
> Subject: Re: [PATCH] AArch64: add GAS support for UDF instruction
> 
> Hi Tamar, Hi Alex,
> > Thanks for fixing it, this looks good to me but you still need a maintainer to
> approve.
> 
> I encountered lots of new testsuite failures when I tested this patch.
> At least I think that I tested it - I used "patch_v2.txt".  Was this the correct
> file ?  If not then please can you point me at the correct patch ?
> 
> The failures were mostly connected with the disassembler not disassembling
> correctly.  eg:
> 
>   regexp "^   0:	10000001 	adr	x1, 0 <bar>$"
>   line   "   0:	10000001 	.inst	0x10000001 ; undefined"
>   FAIL: gas/aarch64/adr_1
> 
> (This was from a toolchain configured as aarch64-linux-gnu).  But there were
> also some internal errors as well. eg:
> 
>   failed with: </gas/testsuite/gas/aarch64/advsimd-armv8_3.s: Assembler
> messages:
>   gas/testsuite/gas/aarch64/advsimd-armv8_3.s:60: Internal error
> (Segmentation fault).
>   Please report this bug.>, no expected output
>   FAIL: gas/aarch64/advsimd-armv8_3
> 
> Cheers
>   Nick


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] AArch64: add GAS support for UDF instruction
  2020-04-30  9:30           ` Tamar Christina
@ 2020-04-30 11:50             ` Nick Clifton
  2020-04-30 14:49               ` Nick Clifton
  0 siblings, 1 reply; 9+ messages in thread
From: Nick Clifton @ 2020-04-30 11:50 UTC (permalink / raw)
  To: Tamar Christina, Alex Coplan
  Cc: binutils, Richard Earnshaw, nd, Marcus Shawcroft

Hi Tamar,

> Just to double check, did you remember to regenerate the -2.c files?

Err ... no. :-(

> Alex's patch omitted them and your error looks consistent with it using an out of sync definition.

Doh.  OK, I will try it again...

Cheers
  Nick


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] AArch64: add GAS support for UDF instruction
  2020-04-30 11:50             ` Nick Clifton
@ 2020-04-30 14:49               ` Nick Clifton
  0 siblings, 0 replies; 9+ messages in thread
From: Nick Clifton @ 2020-04-30 14:49 UTC (permalink / raw)
  To: Tamar Christina, Alex Coplan
  Cc: Richard Earnshaw, nd, binutils, Marcus Shawcroft

Hi Alex,

  Right - sorry about that - the v2 patch is fine and I have gone ahead and applied it.

Cheers
  Nick 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-04-30 14:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-28 10:42 [PATCH] AArch64: add GAS support for UDF instruction Alex Coplan
2020-04-29  7:44 ` Tamar Christina
2020-04-29 15:01   ` Alex Coplan
2020-04-29 15:09     ` Alex Coplan
2020-04-29 19:29       ` Tamar Christina
2020-04-30  9:25         ` Nick Clifton
2020-04-30  9:30           ` Tamar Christina
2020-04-30 11:50             ` Nick Clifton
2020-04-30 14:49               ` Nick Clifton

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