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

[Sudakshina Das]

On 02/11/18 16:09, Sudakshina Das wrote:
> Hi Richard
> 
> On 30/10/18 10:15, Richard Earnshaw (lists) wrote:
>> 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).
>>
> 
> As answered before on patch 2/8
> Thanks you for test case suggestions. I have also updated the tests and
> added a test for the illegal tests. The illegal testing has also
> allowed me to fish out a couple of bugs.
> 
> I have changed the general format of the positive tests to make it
> better to read. All macros are moved on top and every instruction has
> the following testing pattern:
> 1) Base test with x0 and #0 depending on the instruction
> 2) Macro call to add some random tests of different combinations.
> 3) Special casing some SP register and immediate corner cases depending
> on the instruction.
> 
>> R.
>>
> *** 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: Regenerated.
>      * 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-memtag.s: Add tests for ldg.
>      * testsuite/gas/aarch64/armv8_5-a-memtag.d: Likewise.
>      * testsuite/gas/aarch64/illegal-memtag.s: Likewise.
>      * testsuite/gas/aarch64/illegal-memtag.l: Likewise.
> 
> Thanks
> Sudi
> 
New patch. Changelog still applies.

Sudi

> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: rb10009.patch
Type: text/x-patch
Size: 4952 bytes
Desc: rb10009.patch
URL: <https://sourceware.org/pipermail/binutils/attachments/20181108/9d829061/attachment.bin>