Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH] LoongArch: Correct the definition of is_branch_ins()
To:     Tiezhu Yang <yangtiezhu@loongson.cn>,
        Huacai Chen <chenhuacai@kernel.org>,
        WANG Xuerui <kernel@xen0n.name>
Cc:     loongarch@lists.linux.dev, linux-kernel@vger.kernel.org
References: <1671006659-23859-1-git-send-email-yangtiezhu@loongson.cn>
 <e89c80c7-53e5-5fec-d66b-4b375aa85d10@loongson.cn>
 <e65ce2d5-a154-6912-ab01-b0dc158acaaa@loongson.cn>
From:   Jinyang He <hejinyang@loongson.cn>
Message-ID: <5e051682-c4fa-64d7-8567-149b5bd7d403@loongson.cn>
Date:   Sat, 17 Dec 2022 09:49:09 +0800
User-Agent: Mozilla/5.0 (X11; Linux loongarch64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <e65ce2d5-a154-6912-ab01-b0dc158acaaa@loongson.cn>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


On 2022-12-16 14:11, Tiezhu Yang wrote:
>
>
> On 12/16/2022 11:18 AM, Jinyang He wrote:
>> Hi, Tiezhu,
>>
>>
>> On 2022-12-14 16:30, Tiezhu Yang wrote:
>>> The current definition of is_branch_ins() is not correct,
>>
>> But the branch instruction opcode only use the high 6 bits,
>
> Yes, I noticed that, the logic result of current code is right,
> but it seems a little strange (only consider reg1i21_format)
> at the first glance, the initial aim of this patch is to make
> it theoretically correct, maybe it is not the best change.
>
> I think we can neglect the instruction formats and check the
> high 6 bits instead, what do you think of the following change?

We defined many instruction format because of variable-width opcode 
field and parameter field. IMHO if there is no way to solve that problem 
really, keeping original codes is better. That depends on the 
maintainers, of course.


Thanks,

Jinyang


>
> diff --git a/arch/loongarch/include/asm/inst.h 
> b/arch/loongarch/include/asm/inst.h
> index c00e151..fd31752 100644
> --- a/arch/loongarch/include/asm/inst.h
> +++ b/arch/loongarch/include/asm/inst.h
> @@ -329,8 +329,8 @@ static inline bool is_pc_ins(union 
> loongarch_instruction *ip)
>
>  static inline bool is_branch_ins(union loongarch_instruction *ip)
>  {
> -       return ip->reg1i21_format.opcode >= beqz_op &&
> -               ip->reg1i21_format.opcode <= bgeu_op;
> +       return ((ip->word >> 26) & 0x3f) >= beqz_op &&
> +               ((ip->word >> 26) & 0x3f) <= bgeu_op;
>  }
>
> Thanks,
> Tiezhu
>
>
>