[V4,4/8] opcodes: aarch64: flags to denote subclasses of arithmetic insns

Message ID 20240701025404.3361349-5-indu.bhagat@oracle.com
State New
Headers
Series Add SCFI support for aarch64 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Indu Bhagat July 1, 2024, 2:54 a.m. UTC
  [Changes in V4]
- Specify subclasses only for those iclasses relevent to SCFI:
  addsub_imm, and addsub_ext
[End of changes in V4]

[No changes in V3]
[New in V2]

Use the three new subclass flags: F_ARITH_ADD, F_ARITH_SUB,
F_ARITH_MOV, to indicate add, sub and mov ops respectively.

opcodes/
    * aarch64-tbl.h: Use the new F_ARITH_* flags.
---
 opcodes/aarch64-tbl.h | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)
  

Comments

Richard Sandiford July 1, 2024, 6:13 p.m. UTC | #1
Indu Bhagat <indu.bhagat@oracle.com> writes:
> [Changes in V4]
> - Specify subclasses only for those iclasses relevent to SCFI:
>   addsub_imm, and addsub_ext
> [End of changes in V4]
>
> [No changes in V3]
> [New in V2]
>
> Use the three new subclass flags: F_ARITH_ADD, F_ARITH_SUB,
> F_ARITH_MOV, to indicate add, sub and mov ops respectively.
>
> opcodes/
>     * aarch64-tbl.h: Use the new F_ARITH_* flags.
> ---
>  opcodes/aarch64-tbl.h | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
> index 6e523db6277..57727254d43 100644
> --- a/opcodes/aarch64-tbl.h
> +++ b/opcodes/aarch64-tbl.h
> @@ -3205,22 +3205,22 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>    CORE_INSN ("sbcs", 0x7a000000, 0x7fe0fc00, addsub_carry, 0, OP3 (Rd, Rn, Rm), QL_I3SAMER, F_HAS_ALIAS | F_SF),
>    CORE_INSN ("ngcs", 0x7a0003e0, 0x7fe0ffe0, addsub_carry, 0, OP2 (Rd, Rm),     QL_I2SAME,  F_ALIAS | F_SF),
>    /* Add/subtract (extended register).  */
> -  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
> -  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
> -  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
> -  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
> -  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
> -  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
> +  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_ADD | F_SF),
> +  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
> +  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
> +  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_SUB | F_SF),
> +  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
> +  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>    /* Add/subtract (immediate).  */
> -  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_HAS_ALIAS | F_SF),
> -  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ALIAS | F_SF),
> -  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
> -  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
> -  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_SF),
> -  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
> -  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
> -  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
> -  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
> +  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
> +  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ARITH_MOV | F_ALIAS | F_SF),
> +  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
> +  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
> +  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_SUB | F_SF),
> +  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
> +  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
> +  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_ADD),
> +  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_SUB),

I suppose this raises the question: is GINSN_TYPE_ADD specifically
for address arithmetic, or is it a normal addition?  If it's a
normal addition then ADDG doesn't really fit.  If it's address
arithmetic then it might be worth making the names more explicit.

Thanks,
Richard

>    /* Add/subtract (shifted register).  */
>    CORE_INSN ("add",  0x0b000000, 0x7f200000, addsub_shift, 0, OP3 (Rd, Rn, Rm_SFT), QL_I3SAMER, F_SF),
>    CORE_INSN ("adds", 0x2b000000, 0x7f200000, addsub_shift, 0, OP3 (Rd, Rn, Rm_SFT), QL_I3SAMER, F_HAS_ALIAS | F_SF),
  
Indu Bhagat July 11, 2024, 5:47 a.m. UTC | #2
On 7/1/24 11:13, Richard Sandiford wrote:
> Indu Bhagat <indu.bhagat@oracle.com> writes:
>> [Changes in V4]
>> - Specify subclasses only for those iclasses relevent to SCFI:
>>    addsub_imm, and addsub_ext
>> [End of changes in V4]
>>
>> [No changes in V3]
>> [New in V2]
>>
>> Use the three new subclass flags: F_ARITH_ADD, F_ARITH_SUB,
>> F_ARITH_MOV, to indicate add, sub and mov ops respectively.
>>
>> opcodes/
>>      * aarch64-tbl.h: Use the new F_ARITH_* flags.
>> ---
>>   opcodes/aarch64-tbl.h | 30 +++++++++++++++---------------
>>   1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
>> index 6e523db6277..57727254d43 100644
>> --- a/opcodes/aarch64-tbl.h
>> +++ b/opcodes/aarch64-tbl.h
>> @@ -3205,22 +3205,22 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>>     CORE_INSN ("sbcs", 0x7a000000, 0x7fe0fc00, addsub_carry, 0, OP3 (Rd, Rn, Rm), QL_I3SAMER, F_HAS_ALIAS | F_SF),
>>     CORE_INSN ("ngcs", 0x7a0003e0, 0x7fe0ffe0, addsub_carry, 0, OP2 (Rd, Rm),     QL_I2SAME,  F_ALIAS | F_SF),
>>     /* Add/subtract (extended register).  */
>> -  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>> -  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>> -  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>> -  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>> -  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>> -  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>> +  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_ADD | F_SF),
>> +  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>> +  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>> +  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_SUB | F_SF),
>> +  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>> +  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>     /* Add/subtract (immediate).  */
>> -  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_HAS_ALIAS | F_SF),
>> -  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ALIAS | F_SF),
>> -  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>> -  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>> -  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_SF),
>> -  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>> -  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>> -  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>> -  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>> +  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>> +  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ARITH_MOV | F_ALIAS | F_SF),
>> +  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>> +  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>> +  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_SUB | F_SF),
>> +  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>> +  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>> +  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_ADD),
>> +  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_SUB),
> 
> I suppose this raises the question: is GINSN_TYPE_ADD specifically
> for address arithmetic, or is it a normal addition?  If it's a
> normal addition then ADDG doesn't really fit.  If it's address
> arithmetic then it might be worth making the names more explicit.
> 

For SCFI, we are interested in the insns which may have manipulated 
REG_SP and REG_FP, so the intention is around address arithmetic.

ATM, we generate ginsn for _all_ add, sub (and mov) in the iclass 
addsub_imm, addsub_ext (and movewide..), irrespective of whether the 
destination is REG_SP/REG_FP or not.

IOW, "keep the ginsn creation code not tied to GINSN_GEN_SCFI" has been 
followed where affordable.

I dont have a good new name (F_ADDR_ARITH_* ?);  I personally find 
F_ADDR_ARITH_*  unsuitable because this new name ties the 
subclassification to the current usecase (SCFI and its need to see those 
address arithmetic).  But may be I am overthinking.

If you have a suggestion, please let me know.

Thanks
  
Richard Sandiford July 11, 2024, 12:52 p.m. UTC | #3
Indu Bhagat <indu.bhagat@oracle.com> writes:
> On 7/1/24 11:13, Richard Sandiford wrote:
>> Indu Bhagat <indu.bhagat@oracle.com> writes:
>>> [Changes in V4]
>>> - Specify subclasses only for those iclasses relevent to SCFI:
>>>    addsub_imm, and addsub_ext
>>> [End of changes in V4]
>>>
>>> [No changes in V3]
>>> [New in V2]
>>>
>>> Use the three new subclass flags: F_ARITH_ADD, F_ARITH_SUB,
>>> F_ARITH_MOV, to indicate add, sub and mov ops respectively.
>>>
>>> opcodes/
>>>      * aarch64-tbl.h: Use the new F_ARITH_* flags.
>>> ---
>>>   opcodes/aarch64-tbl.h | 30 +++++++++++++++---------------
>>>   1 file changed, 15 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
>>> index 6e523db6277..57727254d43 100644
>>> --- a/opcodes/aarch64-tbl.h
>>> +++ b/opcodes/aarch64-tbl.h
>>> @@ -3205,22 +3205,22 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>>>     CORE_INSN ("sbcs", 0x7a000000, 0x7fe0fc00, addsub_carry, 0, OP3 (Rd, Rn, Rm), QL_I3SAMER, F_HAS_ALIAS | F_SF),
>>>     CORE_INSN ("ngcs", 0x7a0003e0, 0x7fe0ffe0, addsub_carry, 0, OP2 (Rd, Rm),     QL_I2SAME,  F_ALIAS | F_SF),
>>>     /* Add/subtract (extended register).  */
>>> -  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>>> -  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>>> -  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>>> -  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>>> -  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>>> -  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>>> +  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_ADD | F_SF),
>>> +  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>> +  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>> +  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_SUB | F_SF),
>>> +  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>>> +  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>     /* Add/subtract (immediate).  */
>>> -  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_HAS_ALIAS | F_SF),
>>> -  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ALIAS | F_SF),
>>> -  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>>> -  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>>> -  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_SF),
>>> -  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>>> -  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>>> -  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>>> -  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>>> +  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>> +  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ARITH_MOV | F_ALIAS | F_SF),
>>> +  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>> +  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>> +  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_SUB | F_SF),
>>> +  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>>> +  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>> +  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_ADD),
>>> +  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_SUB),
>> 
>> I suppose this raises the question: is GINSN_TYPE_ADD specifically
>> for address arithmetic, or is it a normal addition?  If it's a
>> normal addition then ADDG doesn't really fit.  If it's address
>> arithmetic then it might be worth making the names more explicit.
>> 
>
> For SCFI, we are interested in the insns which may have manipulated 
> REG_SP and REG_FP, so the intention is around address arithmetic.
>
> ATM, we generate ginsn for _all_ add, sub (and mov) in the iclass 
> addsub_imm, addsub_ext (and movewide..), irrespective of whether the 
> destination is REG_SP/REG_FP or not.
>
> IOW, "keep the ginsn creation code not tied to GINSN_GEN_SCFI" has been 
> followed where affordable.

I think this is useful even for SCFI, since large stack allocations could
be done using intermediate calculations into temporary registers.

> I dont have a good new name (F_ADDR_ARITH_* ?);  I personally find 
> F_ADDR_ARITH_*  unsuitable because this new name ties the 
> subclassification to the current usecase (SCFI and its need to see those 
> address arithmetic).  But may be I am overthinking.
>
> If you have a suggestion, please let me know.

The distinction between a full addition and address addition sounds
like it could be generally useful, beyond just SCFI.  How abot having
both GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR?  Places that are only
interested in address arithmetic can treat both types in the same way.
Types that want natural addition can handle GINSN_TYPE_ADD only.

Thanks,
Richard
  
Indu Bhagat July 11, 2024, 5:58 p.m. UTC | #4
On 7/11/24 05:52, Richard Sandiford wrote:
> Indu Bhagat <indu.bhagat@oracle.com> writes:
>> On 7/1/24 11:13, Richard Sandiford wrote:
>>> Indu Bhagat <indu.bhagat@oracle.com> writes:
>>>> [Changes in V4]
>>>> - Specify subclasses only for those iclasses relevent to SCFI:
>>>>     addsub_imm, and addsub_ext
>>>> [End of changes in V4]
>>>>
>>>> [No changes in V3]
>>>> [New in V2]
>>>>
>>>> Use the three new subclass flags: F_ARITH_ADD, F_ARITH_SUB,
>>>> F_ARITH_MOV, to indicate add, sub and mov ops respectively.
>>>>
>>>> opcodes/
>>>>       * aarch64-tbl.h: Use the new F_ARITH_* flags.
>>>> ---
>>>>    opcodes/aarch64-tbl.h | 30 +++++++++++++++---------------
>>>>    1 file changed, 15 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
>>>> index 6e523db6277..57727254d43 100644
>>>> --- a/opcodes/aarch64-tbl.h
>>>> +++ b/opcodes/aarch64-tbl.h
>>>> @@ -3205,22 +3205,22 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>>>>      CORE_INSN ("sbcs", 0x7a000000, 0x7fe0fc00, addsub_carry, 0, OP3 (Rd, Rn, Rm), QL_I3SAMER, F_HAS_ALIAS | F_SF),
>>>>      CORE_INSN ("ngcs", 0x7a0003e0, 0x7fe0ffe0, addsub_carry, 0, OP2 (Rd, Rm),     QL_I2SAME,  F_ALIAS | F_SF),
>>>>      /* Add/subtract (extended register).  */
>>>> -  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>>>> -  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>>>> -  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>>>> -  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>>>> -  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>>>> -  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>>>> +  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_ADD | F_SF),
>>>> +  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>> +  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>> +  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_SUB | F_SF),
>>>> +  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>>>> +  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>      /* Add/subtract (immediate).  */
>>>> -  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>> -  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ALIAS | F_SF),
>>>> -  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>> -  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>>>> -  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_SF),
>>>> -  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>> -  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>>>> -  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>>>> -  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>>>> +  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>> +  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ARITH_MOV | F_ALIAS | F_SF),
>>>> +  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>> +  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>> +  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_SUB | F_SF),
>>>> +  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>>>> +  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>> +  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_ADD),
>>>> +  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_SUB),
>>>
>>> I suppose this raises the question: is GINSN_TYPE_ADD specifically
>>> for address arithmetic, or is it a normal addition?  If it's a
>>> normal addition then ADDG doesn't really fit.  If it's address
>>> arithmetic then it might be worth making the names more explicit.
>>>
>>


Apologies, my brain tricked me into reading the above as "is F_ARITH_ADD 
specifically for address arithmetic or is it normal addition?" all this 
time until now, and hence, what might appear as a confused reply in the 
previous email.

Hopefully I have not digressed too far.

>> For SCFI, we are interested in the insns which may have manipulated
>> REG_SP and REG_FP, so the intention is around address arithmetic.
>>
>> ATM, we generate ginsn for _all_ add, sub (and mov) in the iclass
>> addsub_imm, addsub_ext (and movewide..), irrespective of whether the
>> destination is REG_SP/REG_FP or not.
>>
>> IOW, "keep the ginsn creation code not tied to GINSN_GEN_SCFI" has been
>> followed where affordable.
> 
> I think this is useful even for SCFI, since large stack allocations could
> be done using intermediate calculations into temporary registers.
> 

Yes to using intermediate calculations into temporary registers.  This 
is true especially for aarch64 where we may see patterns like:

         mov     x16, 4384
         add     sp, sp, x16

(which I would like to support for SCFI/aarch64 next.  Adding support 
for this means enabling some data flow for sp traceability; SCFI does 
not have much of data flow ATM.)

But I am not sure if the GINSN_TYPE_ADD / GINSN_TYPE_ADD_ADDR 
demarcation will be useful for SCFI because: effectively GINSN_TYPE_ADD 
with dest as REG_SP/REG_FP is address arithmetic for SCFI. SCFI can (and 
does) ignore the rest of GINSN_TYPE_ADD / GINSN_TYPE_SUB ops.

>> I dont have a good new name (F_ADDR_ARITH_* ?);  I personally find
>> F_ADDR_ARITH_*  unsuitable because this new name ties the
>> subclassification to the current usecase (SCFI and its need to see those
>> address arithmetic).  But may be I am overthinking.
>>
>> If you have a suggestion, please let me know.
> 
> The distinction between a full addition and address addition sounds
> like it could be generally useful, beyond just SCFI.  How abot having
> both GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR?  Places that are only
> interested in address arithmetic can treat both types in the same way.
> Types that want natural addition can handle GINSN_TYPE_ADD only.
> 

The distinction may be useful for usecases other than SCFI, true; 
depending on the usecase.

I think my nervousness with the explicit demarcation using two type of 
ginsns, GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR now, is on the generation 
side:

  - Wont we simply look at the destination register being REG_SP / 
REG_FP in some cases to pick GINSN_TYPE_ADD_ADDR instead of 
GINSN_TYPE_ADD.  If so, GINSN_TYPE_ADD_ADDR contains no more information 
than GINSN_TYPE_ADD in those cases.

  - Also, for other ISAs, I am not sure ATM if this will make things 
more complicated on the ginsn creation side as other rules to pick 
GINSN_TYPE_ADD_ADDR vs GINSN_TYPE_ADD may be involved. And not all might 
be implementable at the time of ginsn creation (as some def-use analysis 
may be necessary to demarcate them?)

Indu
  
Richard Sandiford July 11, 2024, 6:43 p.m. UTC | #5
Indu Bhagat <indu.bhagat@oracle.com> writes:
> On 7/11/24 05:52, Richard Sandiford wrote:
>> Indu Bhagat <indu.bhagat@oracle.com> writes:
>>> On 7/1/24 11:13, Richard Sandiford wrote:
>>>> Indu Bhagat <indu.bhagat@oracle.com> writes:
>>>>> [Changes in V4]
>>>>> - Specify subclasses only for those iclasses relevent to SCFI:
>>>>>     addsub_imm, and addsub_ext
>>>>> [End of changes in V4]
>>>>>
>>>>> [No changes in V3]
>>>>> [New in V2]
>>>>>
>>>>> Use the three new subclass flags: F_ARITH_ADD, F_ARITH_SUB,
>>>>> F_ARITH_MOV, to indicate add, sub and mov ops respectively.
>>>>>
>>>>> opcodes/
>>>>>       * aarch64-tbl.h: Use the new F_ARITH_* flags.
>>>>> ---
>>>>>    opcodes/aarch64-tbl.h | 30 +++++++++++++++---------------
>>>>>    1 file changed, 15 insertions(+), 15 deletions(-)
>>>>>
>>>>> diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
>>>>> index 6e523db6277..57727254d43 100644
>>>>> --- a/opcodes/aarch64-tbl.h
>>>>> +++ b/opcodes/aarch64-tbl.h
>>>>> @@ -3205,22 +3205,22 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>>>>>      CORE_INSN ("sbcs", 0x7a000000, 0x7fe0fc00, addsub_carry, 0, OP3 (Rd, Rn, Rm), QL_I3SAMER, F_HAS_ALIAS | F_SF),
>>>>>      CORE_INSN ("ngcs", 0x7a0003e0, 0x7fe0ffe0, addsub_carry, 0, OP2 (Rd, Rm),     QL_I2SAME,  F_ALIAS | F_SF),
>>>>>      /* Add/subtract (extended register).  */
>>>>> -  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>>>>> -  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>>>>> -  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>>>>> -  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>>>>> -  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>>>>> -  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>>>>> +  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_ADD | F_SF),
>>>>> +  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>>> +  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>> +  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_SUB | F_SF),
>>>>> +  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>>>>> +  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>>      /* Add/subtract (immediate).  */
>>>>> -  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>>> -  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ALIAS | F_SF),
>>>>> -  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>>> -  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>>>>> -  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_SF),
>>>>> -  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>>> -  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>>>>> -  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>>>>> -  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>>>>> +  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>>> +  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ARITH_MOV | F_ALIAS | F_SF),
>>>>> +  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>>> +  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>> +  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_SUB | F_SF),
>>>>> +  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>>>>> +  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>> +  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_ADD),
>>>>> +  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_SUB),
>>>>
>>>> I suppose this raises the question: is GINSN_TYPE_ADD specifically
>>>> for address arithmetic, or is it a normal addition?  If it's a
>>>> normal addition then ADDG doesn't really fit.  If it's address
>>>> arithmetic then it might be worth making the names more explicit.
>>>>
>>>
>
>
> Apologies, my brain tricked me into reading the above as "is F_ARITH_ADD 
> specifically for address arithmetic or is it normal addition?" all this 
> time until now, and hence, what might appear as a confused reply in the 
> previous email.
>
> Hopefully I have not digressed too far.
>
>>> For SCFI, we are interested in the insns which may have manipulated
>>> REG_SP and REG_FP, so the intention is around address arithmetic.
>>>
>>> ATM, we generate ginsn for _all_ add, sub (and mov) in the iclass
>>> addsub_imm, addsub_ext (and movewide..), irrespective of whether the
>>> destination is REG_SP/REG_FP or not.
>>>
>>> IOW, "keep the ginsn creation code not tied to GINSN_GEN_SCFI" has been
>>> followed where affordable.
>> 
>> I think this is useful even for SCFI, since large stack allocations could
>> be done using intermediate calculations into temporary registers.
>> 
>
> Yes to using intermediate calculations into temporary registers.  This 
> is true especially for aarch64 where we may see patterns like:
>
>          mov     x16, 4384
>          add     sp, sp, x16
>
> (which I would like to support for SCFI/aarch64 next.  Adding support 
> for this means enabling some data flow for sp traceability; SCFI does 
> not have much of data flow ATM.)
>
> But I am not sure if the GINSN_TYPE_ADD / GINSN_TYPE_ADD_ADDR 
> demarcation will be useful for SCFI because: effectively GINSN_TYPE_ADD 
> with dest as REG_SP/REG_FP is address arithmetic for SCFI. SCFI can (and 
> does) ignore the rest of GINSN_TYPE_ADD / GINSN_TYPE_SUB ops.
>
>>> I dont have a good new name (F_ADDR_ARITH_* ?);  I personally find
>>> F_ADDR_ARITH_*  unsuitable because this new name ties the
>>> subclassification to the current usecase (SCFI and its need to see those
>>> address arithmetic).  But may be I am overthinking.
>>>
>>> If you have a suggestion, please let me know.
>> 
>> The distinction between a full addition and address addition sounds
>> like it could be generally useful, beyond just SCFI.  How abot having
>> both GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR?  Places that are only
>> interested in address arithmetic can treat both types in the same way.
>> Types that want natural addition can handle GINSN_TYPE_ADD only.
>> 
>
> The distinction may be useful for usecases other than SCFI, true; 
> depending on the usecase.
>
> I think my nervousness with the explicit demarcation using two type of 
> ginsns, GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR now, is on the generation 
> side:
>
>   - Wont we simply look at the destination register being REG_SP / 
> REG_FP in some cases to pick GINSN_TYPE_ADD_ADDR instead of 
> GINSN_TYPE_ADD.  If so, GINSN_TYPE_ADD_ADDR contains no more information 
> than GINSN_TYPE_ADD in those cases.

I was thinking the distinction between GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR
would be based on the opcode rather than the register.  ADDG would be
GINSN_TYPE_ADD_ADDR (since it adds "enough bits" to support address
arithmetic, but does not carry into the tag bits).  ADD would instead be
GINSN_TYPE_ADD (since it's a full-register add).  Both mappings would be
independent of the destination register.

I'm just nervous about saying that ADD and ADDG are the same operation,
unless that operation is specifically described as being for addresses only.

Another option would be to leave ADDG and SUBG as "other" for the
initial patch and deal with them as a follow-on.

>   - Also, for other ISAs, I am not sure ATM if this will make things 
> more complicated on the ginsn creation side as other rules to pick 
> GINSN_TYPE_ADD_ADDR vs GINSN_TYPE_ADD may be involved. And not all might 
> be implementable at the time of ginsn creation (as some def-use analysis 
> may be necessary to demarcate them?)

I think all other ISAs would just use GINSN_TYPE_ADD, unless they're
doing a similar tag operation to ADDG/SUBG.

Thanks,
Richard
  
Indu Bhagat July 12, 2024, 12:53 p.m. UTC | #6
On 7/11/24 11:43, Richard Sandiford wrote:
> Indu Bhagat <indu.bhagat@oracle.com> writes:
>> On 7/11/24 05:52, Richard Sandiford wrote:
>>> Indu Bhagat <indu.bhagat@oracle.com> writes:
>>>> On 7/1/24 11:13, Richard Sandiford wrote:
>>>>> Indu Bhagat <indu.bhagat@oracle.com> writes:
>>>>>> [Changes in V4]
>>>>>> - Specify subclasses only for those iclasses relevent to SCFI:
>>>>>>      addsub_imm, and addsub_ext
>>>>>> [End of changes in V4]
>>>>>>
>>>>>> [No changes in V3]
>>>>>> [New in V2]
>>>>>>
>>>>>> Use the three new subclass flags: F_ARITH_ADD, F_ARITH_SUB,
>>>>>> F_ARITH_MOV, to indicate add, sub and mov ops respectively.
>>>>>>
>>>>>> opcodes/
>>>>>>        * aarch64-tbl.h: Use the new F_ARITH_* flags.
>>>>>> ---
>>>>>>     opcodes/aarch64-tbl.h | 30 +++++++++++++++---------------
>>>>>>     1 file changed, 15 insertions(+), 15 deletions(-)
>>>>>>
>>>>>> diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
>>>>>> index 6e523db6277..57727254d43 100644
>>>>>> --- a/opcodes/aarch64-tbl.h
>>>>>> +++ b/opcodes/aarch64-tbl.h
>>>>>> @@ -3205,22 +3205,22 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>>>>>>       CORE_INSN ("sbcs", 0x7a000000, 0x7fe0fc00, addsub_carry, 0, OP3 (Rd, Rn, Rm), QL_I3SAMER, F_HAS_ALIAS | F_SF),
>>>>>>       CORE_INSN ("ngcs", 0x7a0003e0, 0x7fe0ffe0, addsub_carry, 0, OP2 (Rd, Rm),     QL_I2SAME,  F_ALIAS | F_SF),
>>>>>>       /* Add/subtract (extended register).  */
>>>>>> -  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>>>>>> -  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>>>>>> -  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>>>>>> -  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
>>>>>> -  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
>>>>>> -  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_ADD | F_SF),
>>>>>> +  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_SUB | F_SF),
>>>>>> +  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>>>       /* Add/subtract (immediate).  */
>>>>>> -  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>>>> -  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ALIAS | F_SF),
>>>>>> -  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>>>> -  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>>>>>> -  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_SF),
>>>>>> -  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
>>>>>> -  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
>>>>>> -  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>>>>>> -  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
>>>>>> +  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ARITH_MOV | F_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_SUB | F_SF),
>>>>>> +  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
>>>>>> +  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
>>>>>> +  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_ADD),
>>>>>> +  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_SUB),
>>>>>
>>>>> I suppose this raises the question: is GINSN_TYPE_ADD specifically
>>>>> for address arithmetic, or is it a normal addition?  If it's a
>>>>> normal addition then ADDG doesn't really fit.  If it's address
>>>>> arithmetic then it might be worth making the names more explicit.
>>>>>
>>>>
>>
>>
>> Apologies, my brain tricked me into reading the above as "is F_ARITH_ADD
>> specifically for address arithmetic or is it normal addition?" all this
>> time until now, and hence, what might appear as a confused reply in the
>> previous email.
>>
>> Hopefully I have not digressed too far.
>>
>>>> For SCFI, we are interested in the insns which may have manipulated
>>>> REG_SP and REG_FP, so the intention is around address arithmetic.
>>>>
>>>> ATM, we generate ginsn for _all_ add, sub (and mov) in the iclass
>>>> addsub_imm, addsub_ext (and movewide..), irrespective of whether the
>>>> destination is REG_SP/REG_FP or not.
>>>>
>>>> IOW, "keep the ginsn creation code not tied to GINSN_GEN_SCFI" has been
>>>> followed where affordable.
>>>
>>> I think this is useful even for SCFI, since large stack allocations could
>>> be done using intermediate calculations into temporary registers.
>>>
>>
>> Yes to using intermediate calculations into temporary registers.  This
>> is true especially for aarch64 where we may see patterns like:
>>
>>           mov     x16, 4384
>>           add     sp, sp, x16
>>
>> (which I would like to support for SCFI/aarch64 next.  Adding support
>> for this means enabling some data flow for sp traceability; SCFI does
>> not have much of data flow ATM.)
>>
>> But I am not sure if the GINSN_TYPE_ADD / GINSN_TYPE_ADD_ADDR
>> demarcation will be useful for SCFI because: effectively GINSN_TYPE_ADD
>> with dest as REG_SP/REG_FP is address arithmetic for SCFI. SCFI can (and
>> does) ignore the rest of GINSN_TYPE_ADD / GINSN_TYPE_SUB ops.
>>
>>>> I dont have a good new name (F_ADDR_ARITH_* ?);  I personally find
>>>> F_ADDR_ARITH_*  unsuitable because this new name ties the
>>>> subclassification to the current usecase (SCFI and its need to see those
>>>> address arithmetic).  But may be I am overthinking.
>>>>
>>>> If you have a suggestion, please let me know.
>>>
>>> The distinction between a full addition and address addition sounds
>>> like it could be generally useful, beyond just SCFI.  How abot having
>>> both GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR?  Places that are only
>>> interested in address arithmetic can treat both types in the same way.
>>> Types that want natural addition can handle GINSN_TYPE_ADD only.
>>>
>>
>> The distinction may be useful for usecases other than SCFI, true;
>> depending on the usecase.
>>
>> I think my nervousness with the explicit demarcation using two type of
>> ginsns, GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR now, is on the generation
>> side:
>>
>>    - Wont we simply look at the destination register being REG_SP /
>> REG_FP in some cases to pick GINSN_TYPE_ADD_ADDR instead of
>> GINSN_TYPE_ADD.  If so, GINSN_TYPE_ADD_ADDR contains no more information
>> than GINSN_TYPE_ADD in those cases.
> 
> I was thinking the distinction between GINSN_TYPE_ADD and GINSN_TYPE_ADD_ADDR
> would be based on the opcode rather than the register.  ADDG would be
> GINSN_TYPE_ADD_ADDR (since it adds "enough bits" to support address
> arithmetic, but does not carry into the tag bits).  ADD would instead be
> GINSN_TYPE_ADD (since it's a full-register add).  Both mappings would be
> independent of the destination register.
> 
> I'm just nervous about saying that ADD and ADDG are the same operation,
> unless that operation is specifically described as being for addresses only.
> 
> Another option would be to leave ADDG and SUBG as "other" for the
> initial patch and deal with them as a follow-on.
> 

Right. Your concern is valid.  ADDG/SUBG should not be tagged as 
F_ARITH_ADD/F_ARITH_SUB.  I have changed these to F_SUBCLASS_OTHER.

In hindsight, in V4, tc-aarch64-ginsn.c would generate vanilla 
GINSN_TYPE_ADD/GINSN_TYPE_SUB with no regard to the _scaling_ (As per 
specification: "ADDG adds an immediate value scaled by the Tag granule 
to the address in the source register").

Now in V5, we will generate GINSN_TYPE_OTHER (via the 
aarch64_ginsn_unhandled () codepath) if an addg/subg is seen to affect 
correctness.

>>    - Also, for other ISAs, I am not sure ATM if this will make things
>> more complicated on the ginsn creation side as other rules to pick
>> GINSN_TYPE_ADD_ADDR vs GINSN_TYPE_ADD may be involved. And not all might
>> be implementable at the time of ginsn creation (as some def-use analysis
>> may be necessary to demarcate them?)
> 
> I think all other ISAs would just use GINSN_TYPE_ADD, unless they're
> doing a similar tag operation to ADDG/SUBG.
> 
> Thanks,
> Richard
  

Patch

diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
index 6e523db6277..57727254d43 100644
--- a/opcodes/aarch64-tbl.h
+++ b/opcodes/aarch64-tbl.h
@@ -3205,22 +3205,22 @@  const struct aarch64_opcode aarch64_opcode_table[] =
   CORE_INSN ("sbcs", 0x7a000000, 0x7fe0fc00, addsub_carry, 0, OP3 (Rd, Rn, Rm), QL_I3SAMER, F_HAS_ALIAS | F_SF),
   CORE_INSN ("ngcs", 0x7a0003e0, 0x7fe0ffe0, addsub_carry, 0, OP2 (Rd, Rm),     QL_I2SAME,  F_ALIAS | F_SF),
   /* Add/subtract (extended register).  */
-  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
-  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
-  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
-  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_SF),
-  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_HAS_ALIAS | F_SF),
-  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_ALIAS | F_SF),
+  CORE_INSN ("add",  0x0b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_ADD | F_SF),
+  CORE_INSN ("adds", 0x2b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
+  CORE_INSN ("cmn",  0x2b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
+  CORE_INSN ("sub",  0x4b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd_SP, Rn_SP, Rm_EXT), QL_I3_EXT, F_ARITH_SUB | F_SF),
+  CORE_INSN ("subs", 0x6b200000, 0x7fe00000, addsub_ext, 0, OP3 (Rd, Rn_SP, Rm_EXT),    QL_I3_EXT, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
+  CORE_INSN ("cmp",  0x6b20001f, 0x7fe0001f, addsub_ext, 0, OP2 (Rn_SP, Rm_EXT),        QL_I2_EXT, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
   /* Add/subtract (immediate).  */
-  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_HAS_ALIAS | F_SF),
-  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ALIAS | F_SF),
-  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
-  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
-  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_SF),
-  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_HAS_ALIAS | F_SF),
-  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_ALIAS | F_SF),
-  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
-  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, 0),
+  CORE_INSN ("add",  0x11000000, 0x7f000000, addsub_imm, OP_ADD, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
+  CORE_INSN ("mov",  0x11000000, 0x7ffffc00, addsub_imm, 0, OP2 (Rd_SP, Rn_SP),       QL_I2SP, F_ARITH_MOV | F_ALIAS | F_SF),
+  CORE_INSN ("adds", 0x31000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_ADD | F_HAS_ALIAS | F_SF),
+  CORE_INSN ("cmn",  0x3100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
+  CORE_INSN ("sub",  0x51000000, 0x7f000000, addsub_imm, 0, OP3 (Rd_SP, Rn_SP, AIMM), QL_R2NIL, F_ARITH_SUB | F_SF),
+  CORE_INSN ("subs", 0x71000000, 0x7f000000, addsub_imm, 0, OP3 (Rd, Rn_SP, AIMM),    QL_R2NIL, F_ARITH_SUB | F_HAS_ALIAS | F_SF),
+  CORE_INSN ("cmp",  0x7100001f, 0x7f00001f, addsub_imm, 0, OP2 (Rn_SP, AIMM),        QL_R1NIL, F_SUBCLASS_OTHER | F_ALIAS | F_SF),
+  MEMTAG_INSN ("addg",  0x91800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_ADD),
+  MEMTAG_INSN ("subg",  0xd1800000, 0xffc0c000, addsub_imm, OP4 (Rd_SP, Rn_SP, UIMM10, UIMM4_ADDG), QL_ADDG, F_ARITH_SUB),
   /* Add/subtract (shifted register).  */
   CORE_INSN ("add",  0x0b000000, 0x7f200000, addsub_shift, 0, OP3 (Rd, Rn, Rm_SFT), QL_I3SAMER, F_SF),
   CORE_INSN ("adds", 0x2b000000, 0x7f200000, addsub_shift, 0, OP3 (Rd, Rn, Rm_SFT), QL_I3SAMER, F_HAS_ALIAS | F_SF),