lkml.org 
[lkml]   [2022]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 16/16] objtool/powerpc: Add --mcount specific implementation
Segher Boessenkool wrote:
> On Wed, Aug 31, 2022 at 12:50:07PM +0000, Christophe Leroy wrote:
>> Le 29/08/2022 à 07:52, Sathvika Vasireddy a écrit :
>> > + opcode = insn >> 26;
>> > +
>> > + switch (opcode) {
>> > + case 18: /* bl */
>>
>> case 18 is more than 'bl', it includes also 'b'.
>> In both cases, the calculation of *immediate is the same.
>
> It also is "ba" and "bla", sometimes written as "b[l][a]".
>
>> It would therefore be more correct to perform the calculation and setup
>> of *immediate outside the 'if ((insn & 3) == 1)', that would avoid
>> 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) == 1) { /* bl */

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

The code does test for AA=0 LK=1 with the if statement there?


- Naveen

\
 
 \ /
  Last update: 2022-09-05 12:47    [W:0.159 / U:0.472 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site