2021-01-12 13:07:59

by Jinyang He

[permalink] [raw]
Subject: [PATCH 4/4] MIPS: Add is_jr_ra_ins() to end the loop early

For those leaf functions, they are likely to have no stack operations.
Add is_jr_ra_ins() to determine whether jr ra has been touched before
the frame_size is found. Without this patch, the get frame_size operation
may be out of range and get the frame_size from the next nested function.

Signed-off-by: Jinyang He <[email protected]>
---
arch/mips/kernel/process.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index bef8f8d..9e6f194 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -205,6 +205,36 @@ struct mips_frame_info {
#define J_TARGET(pc,target) \
(((unsigned long)(pc) & 0xf0000000) | ((target) << 2))

+static inline int is_jr_ra_ins(union mips_instruction *ip)
+{
+#ifdef CONFIG_CPU_MICROMIPS
+ /*
+ * jr16 ra
+ * jr ra
+ */
+ if (mm_insn_16bit(ip->word >> 16)) {
+ if (ip->mm16_r5_format.opcode == mm_pool16c_op &&
+ ip->mm16_r5_format.rt == mm_jr16_op &&
+ ip->mm16_r5_format.imm == 31)
+ return 1;
+ return 0;
+ }
+
+ if (ip->r_format.opcode == mm_pool32a_op &&
+ ip->r_format.func == mm_pool32axf_op &&
+ ((ip->u_format.uimmediate >> 6) & GENMASK(9,0)) == mm_jalr_op &&
+ ip->r_format.rs == 31)
+ return 1;
+ return 0;
+#else
+ if (ip->r_format.opcode == spec_op &&
+ ip->r_format.func == jr_op &&
+ ip->r_format.rs == 31)
+ return 1;
+ return 0;
+#endif
+}
+
static inline int is_ra_save_ins(union mips_instruction *ip, int *poff)
{
#ifdef CONFIG_CPU_MICROMIPS
@@ -417,7 +447,9 @@ static int get_frame_info(struct mips_frame_info *info)
last_insn_size = 4;
}

- if (!info->frame_size) {
+ if (is_jr_ra_ins(ip)) {
+ break;
+ } else if (!info->frame_size) {
is_sp_move_ins(&insn, &info->frame_size);
continue;
} else if (!saw_jump && is_jump_ins(ip)) {
--
2.1.0


2021-10-23 18:39:47

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH 4/4] MIPS: Add is_jr_ra_ins() to end the loop early

On Tue, 12 Jan 2021, Jinyang He wrote:

> For those leaf functions, they are likely to have no stack operations.
> Add is_jr_ra_ins() to determine whether jr ra has been touched before
> the frame_size is found. Without this patch, the get frame_size operation
> may be out of range and get the frame_size from the next nested function.
>
> Signed-off-by: Jinyang He <[email protected]>
> ---
> arch/mips/kernel/process.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
> index bef8f8d..9e6f194 100644
> --- a/arch/mips/kernel/process.c
> +++ b/arch/mips/kernel/process.c
> @@ -205,6 +205,36 @@ struct mips_frame_info {
> #define J_TARGET(pc,target) \
> (((unsigned long)(pc) & 0xf0000000) | ((target) << 2))
>
> +static inline int is_jr_ra_ins(union mips_instruction *ip)
> +{
> +#ifdef CONFIG_CPU_MICROMIPS
> + /*
> + * jr16 ra
> + * jr ra
> + */
> + if (mm_insn_16bit(ip->word >> 16)) {
> + if (ip->mm16_r5_format.opcode == mm_pool16c_op &&
> + ip->mm16_r5_format.rt == mm_jr16_op &&
> + ip->mm16_r5_format.imm == 31)
> + return 1;
> + return 0;
> + }
> +
> + if (ip->r_format.opcode == mm_pool32a_op &&
> + ip->r_format.func == mm_pool32axf_op &&
> + ((ip->u_format.uimmediate >> 6) & GENMASK(9,0)) == mm_jalr_op &&
> + ip->r_format.rs == 31)
> + return 1;
> + return 0;
> +#else

Without looking into it much, this is likely missing the point, because
while technically inteed JR and JR16 can be used with $ra in microMIPS
machine code (there's JRS too), in reality either JRC or JRADDIUSP will.

[Wading through e-mail recovered from mid-Jan linux-mips.org crash.]

Maciej

2021-10-25 02:21:06

by Jinyang He

[permalink] [raw]
Subject: Re: [PATCH 4/4] MIPS: Add is_jr_ra_ins() to end the loop early

On 10/24/2021 02:32 AM, Maciej W. Rozycki wrote:

> On Tue, 12 Jan 2021, Jinyang He wrote:
>
>> For those leaf functions, they are likely to have no stack operations.
>> Add is_jr_ra_ins() to determine whether jr ra has been touched before
>> the frame_size is found. Without this patch, the get frame_size operation
>> may be out of range and get the frame_size from the next nested function.
>>
>> Signed-off-by: Jinyang He <[email protected]>
>> ---
>> arch/mips/kernel/process.c | 34 +++++++++++++++++++++++++++++++++-
>> 1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
>> index bef8f8d..9e6f194 100644
>> --- a/arch/mips/kernel/process.c
>> +++ b/arch/mips/kernel/process.c
>> @@ -205,6 +205,36 @@ struct mips_frame_info {
>> #define J_TARGET(pc,target) \
>> (((unsigned long)(pc) & 0xf0000000) | ((target) << 2))
>>
>> +static inline int is_jr_ra_ins(union mips_instruction *ip)
>> +{
>> +#ifdef CONFIG_CPU_MICROMIPS
>> + /*
>> + * jr16 ra
>> + * jr ra
>> + */
>> + if (mm_insn_16bit(ip->word >> 16)) {
>> + if (ip->mm16_r5_format.opcode == mm_pool16c_op &&
>> + ip->mm16_r5_format.rt == mm_jr16_op &&
>> + ip->mm16_r5_format.imm == 31)
>> + return 1;
>> + return 0;
>> + }
>> +
>> + if (ip->r_format.opcode == mm_pool32a_op &&
>> + ip->r_format.func == mm_pool32axf_op &&
>> + ((ip->u_format.uimmediate >> 6) & GENMASK(9,0)) == mm_jalr_op &&
>> + ip->r_format.rs == 31)
>> + return 1;
>> + return 0;
>> +#else
> Without looking into it much, this is likely missing the point, because
> while technically inteed JR and JR16 can be used with $ra in microMIPS
> machine code (there's JRS too), in reality either JRC or JRADDIUSP will.
>
> [Wading through e-mail recovered from mid-Jan linux-mips.org crash.]
>
> Maciej

You are right, I missed the point. I have refered to microMIPS64
Instruction Set. JRC and JRADDIUSP also can be used with $ra in
microMIPS. I am not sure what compiler do at epilogue. But when
we call $ra without saving $ra, it means return, I think.
I will fix it later.

Thanks,
Jinyang.