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 433293838A04 for ; Thu, 27 Jun 2024 09:40:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 433293838A04 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 433293838A04 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=1719481247; cv=none; b=J05qXTyvt/5x/zdFW/B/rZD9rVeyImWgqF1u3OlvA2Cne2c1rb1XOA8fgPkqvMHHCI0PreDWp6z4w8exZXhPjUKE/P03MKdfgV9o3cD7nnNylxvjo2Qi09di4O5glb06tS8w2znoQQdOycM5ddm+Nl8PPNPcA3syQzGKHghB9gA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719481247; c=relaxed/simple; bh=1TP/SGOxYBU+MG4HESmv7It9RycekNiPmo08erE1TgI=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=D3/LXA7kdQEY3q82As3hjURZJr37FCuIWWunv6/yPPlcpIl9gtSEFUqkz/WsoqhvTxEtmJKlggvMDQHf7A2+UofKFlNjLcsoroxfWNNt2Z44G2MA5h0M1zVWMtUyENFrgLXbW9S6HkEJLl/lOezf24stPDeC6kbdV0pUVNfbrPQ= 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 6275E367; Thu, 27 Jun 2024 02:41:08 -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 F34703F766; Thu, 27 Jun 2024 02:40:42 -0700 (PDT) From: Richard Sandiford To: Indu Bhagat Mail-Followup-To: Indu Bhagat ,binutils@sourceware.org, richard.sandiford@arm.com Cc: binutils@sourceware.org Subject: Re: [PATCH 0/2] Add SCFI support for aarch64 In-Reply-To: <793ecdde-91bb-401d-87c3-5bd40fbd3c20@oracle.com> (Indu Bhagat's message of "Thu, 27 Jun 2024 01:00:53 -0700") References: <20240411074407.1429624-1-indu.bhagat@oracle.com> <793ecdde-91bb-401d-87c3-5bd40fbd3c20@oracle.com> Date: Thu, 27 Jun 2024 10:40:41 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_NUMSUBJECT,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no 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 6/26/24 04:01, Richard Sandiford wrote: >> D8-D15 are "interesting" because they are the low 64 bits of Q8-Q15, >> and of Z8-Z15 if SVE is used. However, a CFI save slot always represents >> the low 64 bits, regardless of whether a save occurs on D, Q or Z registers. >> This matters for big-endian code, because there are two additional >> PCS variants: >> >> * the "vector PCS", which preserves Q8-Q23 >> * the "SVE PCS", which preserves Z8-Z23 and P3-P15 >> > > Is there a way to annotate that a (hand-written asm) function adheres to > vectors PCS or SVE PCS ? I see that there is a .variant_pcs but that > does not help differentiate between the above two? > > I _think_ gas will need to know which of SVE vs vector PCS is in effect > for a specific function so that the P3-P15 can be added to the set of > callee-saved registers being tracked for SCFI for SVE PCS but not for > vector PCS. Only the normal base AAPCS64 register set is preserved across abnormal control flow (setjmp/longjmp, exceptions, etc.) The extra call-preserved guarantees for vector and SVE PCS functions only apply to normal returns. [This means, for example, that: void foo(); svbool_t f() { try { foo(); } catch (...) {}; return svptrue_b8(); } must manually restore the additional register state when catching and returning normally.] The CFI requirements therefore don't change: only D8-D15 matter, like for normal functions. But that's also where the big-endian complications that I mentioned come from. So I don't think the code needs to know which kind of function is being assembled. The code just needs to be able to recognise Q-based and Z-based loads and stores of D8-D15 and work out the correct offset of the low 64 bits. (Although, like I say, I think we can punt on big-endian SVE PCS functions.) >> So vector PCS functions might need to save and restore Q8 when returning >> normally, but the CFI only describes the save of the D8 portion (since >> that's the only portion that is preserved by exceptions). This means >> that, on big-endian: >> >> str q8, [sp, #16] >> >> should record D8 as being saved at sp+24 rather than sp+16. >> >> A further complication is that STR Qn and STR Zn do not store in >> the same byte order for big-endian: STR Qn stores as a 128-bit >> integer (MSB first), whereas STR Zn stores as a stream of bytes >> (LSB first). This means that GCC-generated big-endian SVE PCS >> functions use things like: >> >> st1d z8.d, p2, [sp, #1, mul vl] >> >> with the D8 save slot then being at sp + 2*VL - 64. >> >> I think it's OK to punt on the big-endian SVE PCS case for now (provided >> that there's a warning that the code isn't understood, which it looks >> like there is). But I think it's worth handling the Q register saves. > > It looks to me that using reg name / size is an unambiguous proxy to > deciding whether SVE PCS is in effect. Is this correct ? Not necessarily. There's nothing stopping code from using Q-based loads and stores for normal functions (although it would be an odd choice). There's also the possiblity of ad-hoc PCSes, but the assumption there too would be that only the base AAPCS64 set needs to be preserved through unwinding. >> Other comments: >> >> - I like the new approach of using a combination of the iclass and a >> "subclass" field of the flags. How about making aarch64-gen.c enforce >> that: >> >> - if aarch64-ginsn.c looks at the subclass of a particular iclass, >> every instruction of that iclass has a nonzero subclass field >> > > (Let me refer to the above as #1). I can see that there can be ways to > achieve this... > >> - every other instruction has a zero subclass field >> > > ..but I am not sure I follow this statement. (Let me refer to the above > as #2). > >> This would help to ensure that the data stays up to date. >> The subclass enum could include a nonzero "other" value where >> necessary. >> > > Currently, we are using the opcode->flags bits to encode: > > In include/opcode/aarch64.h: > > /* 4-bit flag field to indicate subclass of operations. > Note that there is an (intended) overlap between the three flag sets > (F_LDST*, F_ARITH* and F_BRANCH*). This allows space savings. */ > #define F_LDST_LOAD (1ULL << 36) > #define F_LDST_STORE (2ULL << 36) > /* A load followed by a store (using the same address). */ > #define F_LDST_SWAP (F_LDST_LOAD | F_LDST_STORE) > /* Subclasses to denote add, sub and mov insns. */ > #define F_ARITH_ADD (1ULL << 36) > #define F_ARITH_SUB (2ULL << 36) > #define F_ARITH_MOV (4ULL << 36) > /* Subclasses to denote call and ret insns. */ > #define F_BRANCH_CALL (1ULL << 36) > #define F_BRANCH_RET (2ULL << 36) > > We can dedicate F_SUBCLASS_NONE (8ULL << 36) and enforce this subclass > on all insns which use none of the above subclasses in a specific > iclass. This can help address (#1), but not sure about (#2). I think the 4 bits are really an enum rather than true independent flags. So it might be better to use 15ULL, so that the other 14 nonzero values are consecutive. But yeah, I think it addresses both #1 and #2. #2 makes sure that a subclass is only present when we expect one. If we define: #define F_SUBCLASS (15ULL << 36) then #2 makes sure that (flags & F_SUBCLASS) == 0 for classes that are not interpreted by ginsns. Thanks, Richard