From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id BA7A8384A06E for ; Thu, 11 Jul 2024 12:52:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BA7A8384A06E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BA7A8384A06E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720702353; cv=none; b=Tvs9mMJqpI0KFZ5IBkByDCnDtUJx+FS1Mf+TZgpmxR94Mgzr0hTLkInk5QFMNQ3MHwfdJasVK8aaT7/pFoMYWm2z/+tI8UdxuY0pKo03Mv7lxLwTXL3yGlI5LxPdAs5V+/5pr7MfuIBe5HLYyev0D4JYmv0IkaN3aBVH7P0VX0Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720702353; c=relaxed/simple; bh=bNA9t15ZZAESlWnEaVkkrW5ffxGRp1m8L9GOgMH547w=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=i1vjb0rlj2rx154VJ2Vwk0kOchOWcI+50YdLRFmdPxdyGqnX+FyqVYVchS/WwmSSvbL4sEpfCC66Dg+jYYDsXfeqUsQBSlSAnQJs4B9aeeqD5Q94REzzm5Gaj9EYv77k6wicxxt64B6vygKTW4qDYYeefBfpMSOvPseoFzlBdxU= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B7B071007; Thu, 11 Jul 2024 05:52:55 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F3E423F766; Thu, 11 Jul 2024 05:52:29 -0700 (PDT) From: Richard Sandiford To: Indu Bhagat Mail-Followup-To: Indu Bhagat ,binutils@sourceware.org, Richard.Earnshaw@arm.com, richard.sandiford@arm.com Cc: binutils@sourceware.org, Richard.Earnshaw@arm.com Subject: Re: [PATCH,V4 4/8] opcodes: aarch64: flags to denote subclasses of arithmetic insns In-Reply-To: (Indu Bhagat's message of "Wed, 10 Jul 2024 22:47:28 -0700") References: <20240701025404.3361349-1-indu.bhagat@oracle.com> <20240701025404.3361349-5-indu.bhagat@oracle.com> Date: Thu, 11 Jul 2024 13:52:28 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-19.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Indu Bhagat writes: > On 7/1/24 11:13, Richard Sandiford wrote: >> Indu Bhagat 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