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;
Date:   Mon, 05 Sep 2022 16:15:07 +0530
From:   "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Subject: Re: [PATCH v2 16/16] objtool/powerpc: Add --mcount specific
 implementation
To:     Christophe Leroy <christophe.leroy@csgroup.eu>,
        Segher Boessenkool <segher@kernel.crashing.org>
Cc:     "aik@ozlabs.ru" <aik@ozlabs.ru>,
        "chenzhongjin@huawei.com" <chenzhongjin@huawei.com>,
        "jpoimboe@redhat.com" <jpoimboe@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
        "mbenes@suse.cz" <mbenes@suse.cz>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "npiggin@gmail.com" <npiggin@gmail.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        Sathvika Vasireddy <sv@linux.ibm.com>
References: <20220829055223.24767-1-sv@linux.ibm.com>
        <20220829055223.24767-17-sv@linux.ibm.com>
        <5da86617-53f1-d899-336a-53fe691e76ae@csgroup.eu>
        <20220831175100.GS25951@gate.crashing.org>
In-Reply-To: <20220831175100.GS25951@gate.crashing.org>
MIME-Version: 1.0
User-Agent: astroid/4d6b06ad (https://github.com/astroidmail/astroid)
Message-Id: <1662374555.7eoowv9twf.naveen@linux.ibm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Segher Boessenkool wrote:
> On Wed, Aug 31, 2022 at 12:50:07PM +0000, Christophe Leroy wrote:
>> Le 29/08/2022 =C3=A0 07:52, Sathvika Vasireddy a =C3=A9crit=C2=A0:
>> > +	opcode =3D insn >> 26;
>> > +
>> > +	switch (opcode) {
>> > +	case 18: /* bl */
>>=20
>> case 18 is more than 'bl', it includes also 'b'.
>> In both cases, the calculation of *immediate is the same.
>=20
> It also is "ba" and "bla", sometimes written as "b[l][a]".
>=20
>> It would therefore be more correct to perform the calculation and setup=20
>> of *immediate outside the 'if ((insn & 3) =3D=3D 1)', that would avoid=20
>> unnecessary churn the day we add support for branches without link.

Yeah, and probably move the comments around:

+	case 18: /* b[l][a] */
+		if ((insn & 3) =3D=3D 1) { /* bl */

>>=20
>> > +		if ((insn & 3) =3D=3D 1) {
>> > +			*type =3D INSN_CALL;
>> > +			*immediate =3D insn & 0x3fffffc;
>> > +			if (*immediate & 0x2000000)
>> > +				*immediate -=3D 0x4000000;
>> > +		}
>> > +		break;
>> > +	}
>=20
> Does this handle AA=3D1 correctly at all?  That is valid both with and
> without relocations, just like AA=3D0.  Same for AA=3D1 LK=3D0 btw.
>=20
> If you only handle AA=3D0, the code should explicitly test for that.

The code does test for AA=3D0 LK=3D1 with the if statement there?


- Naveen