Re: [PATCH, BINUTILS, AARCH64, 5/8] Add Tag getting instruction in Memory Tagging Extension

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

On 09/10/2018 18:25, Sudakshina Das wrote:
> Hi
> 
> This patch is part of the patch series to add support for ARMv8.5-A
> Memory Tagging Extensions.
> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)
> 
> Memory Tagging Extension (MTE) is an optional extension to
> ARMv8.5-A and is enabled using the +memtag command line option.
> 
> This patch add support to the Tag Getting instruction from
> MTE:
> - LDG <Xt>, [<Xn|SP>, #<simm>]
> 
> where
> <Xt> : Is the 64-bit destination GPR.
> <Xn|SP> : Is the 64-bit first source GPR or Stack pointer.
> <simm> : Is the optional signed immediate offset, a multiple of 16
> in the range of -4096 and 4080, defaulting to 0.
> 
> Testing done: Builds and reg tests all pass on aarch64-none-linux-gnu.
> Added tests.
> 
> Is this ok for trunk?
> 
> Thanks
> Sudi
> 
> *** opcodes/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
> 
>     * aarch64-tbl.h (QL_LDG): New.
>     (aarch64_opcode_table): Add ldg.
>     * aarch64-asm-2.c: Regenarated.
>     * aarch64-dis-2.c: Regenerated.
>     * aarch64-opc-2.c: Regenerated.
> 
> *** gas/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  <sudi.das@arm.com>
> 
>     * testsuite/gas/aarch64/armv8_5-a-mte.s: Add tests for ldg.
>     * testsuite/gas/aarch64/armv8_5-a-mte.d: Likewise.
> 
> 

Same mte/memtag issue as earlier patches.

What's the rationale behind this set of test instructions?


+.*:	d96001d5 	ldg	x21, \[x14\]
+.*:	d960a1d5 	ldg	x21, \[x14, #160\]
+.*:	d961e1d5 	ldg	x21, \[x14, #480\]
+.*:	d962d1d5 	ldg	x21, \[x14, #720\]
+.*:	d965a1d5 	ldg	x21, \[x14, #1440\]
+.*:	d96c81d5 	ldg	x21, \[x14, #3200\]
+.*:	d96f51d5 	ldg	x21, \[x14, #3920\]
+.*:	d97fb30c 	ldg	x12, \[x24, #-80\]
+.*:	d97ec30c 	ldg	x12, \[x24, #-320\]
+.*:	d97dd30c 	ldg	x12, \[x24, #-560\]
+.*:	d97d330c 	ldg	x12, \[x24, #-720\]
+.*:	d97a530c 	ldg	x12, \[x24, #-1456\]
+.*:	d973730c 	ldg	x12, \[x24, #-3216\]
+.*:	d970330c 	ldg	x12, \[x24, #-4048\]
+.*:	d96ff3e1 	ldg	x1, \[sp, #4080\]
+.*:	d97003eb 	ldg	x11, \[sp, #-4096\]

It doesn't seem particularly scientific to me.

Generally gas tests need to:

- test the base encoding of the instruction (try to find (a minimal set
of) examples where all the operand fields are not set)
- test each operand independently with a minimal set of tests that
ensure the bits are encoded in the correct places.  eg if you have a
contiguous 5-bit field, try to set all the bits in it: if x31/sp, that
is sufficient.  Fields that have multiple insertion points (ie are split
across the instruction need a bit more care).
- a couple of random values, just to cover some other cases - try to
avoid combinations that look too similar to other tests in the suite.

- test that invalid operands are rejected (don't permit xzr/sp if the
instruction should not allow it, reject invalid immediates - don't
forget invalid cases like not-a-multiple-of as well as too large/small).

So tests here should probably be something like:

ldg	x0, [x0]	// base pattern
ldg	x21, [x0]	// sets top and bottom bits of load register
ldg	x0, [sp]	// top-and-bottom of address
ldg	x0, [x0, #-4096] // minimum negative offset
ldg	x0, [x0, #4080] // maximum positive offset
ldg	x8, [x10, #720] // random tests...

ldg	xzr, [x0]	// error, xzr not valid
ldg	x0, [x0, #3]	// error, invalid offset
...

If you have access to another disassembler, then results should be cross
checked with that.  Otherwise each test value needs to be manually
verified for accuracy (otherwise the test doesn't tell us anything).

R.