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

[Richard Sandiford]

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