From: Indu Bhagat <indu.bhagat@oracle.com>
To: binutils@sourceware.org, Richard.Earnshaw@arm.com,
richard.sandiford@arm.com
Subject: Re: [PATCH,V4 2/8] include: opcodes: aarch64: define new subclasses
Date: Wed, 10 Jul 2024 22:14:26 -0700 [thread overview]
Message-ID: <8f282d4f-8402-429c-967d-994ff23114ec@oracle.com> (raw)
In-Reply-To: <mptr0cdko3z.fsf@arm.com>
On 7/1/24 10:40, Richard Sandiford wrote:
> Indu Bhagat <indu.bhagat@oracle.com> writes:
>> [New in V4]
>>
>> The existing iclass information tells us the general shape and purpose
>> of the instructions. In some cases, however, we need to further disect
>> the iclass on the basis of other finer-grain information. E.g., for the
>> purpose of SCFI, we need to know whether a given insn with iclass of
>> ldst_* is a load or a store. Similarly, whether a particular arithmetic
>> insn is an add or sub or mov, etc.
>>
>> This patch defines new flags to demarcate the insns. Also provide an
>> access function for subclass lookup.
>>
>> Later, we will enforce (in aarch64-gen.c) that if an iclass has at least
>> one instruction with a non-zero subclass, all instructions of the iclass
>> must have a non-zero subclass information. If none of the defined
>> subclasses are applicable (or not required for SCFI purposes),
>> F_SUBCLASS_OTHER can be used for such instructions.
>>
>> include/
>> * opcode/aarch64.h (F_SUBCLASS): New flag.
>> (F_SUBCLASS_OTHER): Likewise.
>> (F_LDST_LOAD): Likewise.
>> (F_LDST_STORE): Likewise.
>> (F_LDST_SWAP): Likewise.
>> (F_ARITH_ADD): Likewise.
>> (F_ARITH_SUB): Likewise.
>> (F_ARITH_MOV): Likewise.
>> (F_BRANCH_CALL): Likewise.
>> (F_BRANCH_RET): Likewise.
>> (F_MAX_SUBCLASS): Likewise.
>> (aarch64_opcode_subclass_p): New definition.
>> ---
>> include/opcode/aarch64.h | 32 +++++++++++++++++++++++++++++++-
>> 1 file changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h
>> index 61758c96285..c3ac1326b9c 100644
>> --- a/include/opcode/aarch64.h
>> +++ b/include/opcode/aarch64.h
>> @@ -1359,7 +1359,31 @@ extern const aarch64_opcode aarch64_opcode_table[];
>> #define F_OPD_SIZE (1ULL << 34)
>> /* RCPC3 instruction has the field of 'size'. */
>> #define F_RCPC3_SIZE (1ULL << 35)
>> -/* Next bit is 36. */
>> +
>> +/* 4-bit flag field to indicate subclass of instructions.
>> + The available 14 nonzero values are consecutive, with room for
>> + more subclasses in future. */
>> +#define F_SUBCLASS (15ULL << 36)
>> +
>> +#define F_SUBCLASS_OTHER (F_SUBCLASS)
>> +
>> +#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 (3ULL << 36)
>> +/* Subclasses to denote add, sub and mov insns. */
>> +#define F_ARITH_ADD (4ULL << 36)
>> +#define F_ARITH_SUB (5ULL << 36)
>> +#define F_ARITH_MOV (6ULL << 36)
>> +/* Subclasses to denote call and ret insns. */
>> +#define F_BRANCH_CALL (7ULL << 36)
>> +#define F_BRANCH_RET (8ULL << 36)
>
> When I said this field was an enum, I should have said that it's
> a class-specific enum. I don't think we need the ldst and arith
> subclasses to use independent values, and it might be better to
> allow overlap from the outset, rather than try to retrofit it later.
>
> F_LDST_SWAP is no longer used (a good thing!)
>
OK. I have switched it to iclass-specific enum with overlap. I have
removed F_LDST_SWAP and F_MAX_SUBLASS as they are unused.
I have added function-level comments:
/* Whether the opcode has the specific subclass flag.
N.B. The overlap between F_LDST_*, F_ARITH_*, and F_BRANCH_* flags means
that the callers of this function have the responsibility of
checking for
the flags appropriate for the specific iclass. */
static inline bool
aarch64_opcode_subclass_p (const aarch64_opcode* opcode, uint64_t flag)
{
return ((opcode->flags & F_SUBCLASS) == flag);
}
> Thanks,
> Richard
>
>> +
>> +#define F_SUBCLASS_OTHER (F_SUBCLASS)
>> +/* PS: To perform some sanity checks, the last declared subclass flag is used.
>> + Keep F_MAX_SUBCLASS updated when declaring new subclasses. */
>> +#define F_MAX_SUBCLASS (F_BRANCH_RET)
>> +/* Next bit is 40. */
>>
>> /* Instruction constraints. */
>> /* This instruction has a predication constraint on the instruction at PC+4. */
>> @@ -1398,6 +1422,12 @@ pseudo_opcode_p (const aarch64_opcode *opcode)
>> return (opcode->flags & F_PSEUDO) != 0lu;
>> }
>>
>> +static inline bool
>> +aarch64_opcode_subclass_p (const aarch64_opcode* opcode, uint64_t flag)
>> +{
>> + return ((opcode->flags & F_SUBCLASS) == flag);
>> +}
>> +
>> /* Deal with two possible scenarios: If F_OP_PAIR_OPT not set, as is the case
>> by default, F_OPDn_OPT must equal IDX + 1, else F_OPDn_OPT must be in range
>> [IDX, IDX + 1]. */
next prev parent reply other threads:[~2024-07-11 5:14 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-01 2:53 [PATCH,V4 0/8] Add SCFI support for aarch64 Indu Bhagat
2024-07-01 2:53 ` [PATCH,V4 1/8] gas: scfi: make scfi_state_restore_reg function more precise Indu Bhagat
2024-07-12 15:03 ` [PATCH, V4 " Indu Bhagat
2024-07-01 2:53 ` [PATCH,V4 2/8] include: opcodes: aarch64: define new subclasses Indu Bhagat
2024-07-01 17:40 ` Richard Sandiford
2024-07-11 5:14 ` Indu Bhagat [this message]
2024-07-11 12:22 ` Richard Sandiford
2024-07-11 17:59 ` Indu Bhagat
2024-07-01 2:53 ` [PATCH,V4 3/8] opcodes: aarch64: flags to denote subclasses of ldst insns Indu Bhagat
2024-07-01 18:06 ` Richard Sandiford
2024-07-11 5:45 ` Indu Bhagat
2024-07-12 13:59 ` Indu Bhagat
2024-07-13 7:34 ` Indu Bhagat
2024-07-01 2:54 ` [PATCH,V4 4/8] opcodes: aarch64: flags to denote subclasses of arithmetic insns Indu Bhagat
2024-07-01 18:13 ` Richard Sandiford
2024-07-11 5:47 ` Indu Bhagat
2024-07-11 12:52 ` Richard Sandiford
2024-07-11 17:58 ` Indu Bhagat
2024-07-11 18:43 ` Richard Sandiford
2024-07-12 12:53 ` Indu Bhagat
2024-07-01 2:54 ` [PATCH,V4 5/8] opcodes: aarch64: flags to denote subclasses of uncond branches Indu Bhagat
2024-07-01 2:54 ` [PATCH,V4 6/8] opcodes: aarch64: enforce checks on subclass flags in aarch64-gen.c Indu Bhagat
2024-07-01 2:54 ` [PATCH,V4 7/8] gas: aarch64: add experimental support for SCFI Indu Bhagat
2024-07-01 19:49 ` Richard Sandiford
2024-07-11 6:30 ` Indu Bhagat
2024-07-11 13:15 ` Richard Sandiford
2024-07-11 19:07 ` Indu Bhagat
2024-07-11 20:10 ` Richard Sandiford
2024-07-01 2:54 ` [PATCH,V4 8/8] gas: aarch64: testsuite: add new tests " Indu Bhagat
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8f282d4f-8402-429c-967d-994ff23114ec@oracle.com \
--to=indu.bhagat@oracle.com \
--cc=Richard.Earnshaw@arm.com \
--cc=binutils@sourceware.org \
--cc=richard.sandiford@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).