[PATCH] AArch64: add GAS support for UDF instruction
Tamar Christina
Tamar.Christina@arm.com
Wed Apr 29 19:29:46 GMT 2020
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.
More information about the Binutils
mailing list