Le 01/09/2022 à 00:45, Segher Boessenkool a écrit :
> Hi!
>
> On Tue, Aug 30, 2022 at 09:10:02AM +0000, Christophe Leroy wrote:
>> Le 30/08/2022 à 11:01, Nicholas Piggin a écrit :
>>> On Tue Aug 30, 2022 at 3:24 PM AEST, Christophe Leroy wrote:
>>>>> This is still slightly concerning to me. Is there any guarantee that the
>>>>> compiler would not use a different sequence for the address here?
>>>>>
>>>>> Maybe explicit r13 is required.
>>>>>
>>>>
>>>> local_paca is defined as:
>>>>
>>>> register struct paca_struct *local_paca asm("r13");
>
> And this is in global scope, making it a global register variable.
>
>>>> Why would the compiler use another register ?
>>>
>>> Hopefully it doesn't. Is it guaranteed that it won't?
>
> Yes, this is guaranteed.
>
> For a local register variable this is guaranteed only for operands to an
> extended inline asm; any other access to the variable does not have to
> put it in the specified register.
>
> But this is a global register variable. The only thing that would make
> this crash and burn is if *any* translation unit did not see this
> declaration: it could then use r13 (if that was allowed by the ABI in
> effect, heh).
>
>>> I'm sure Segher will be delighted with the creative asm in __do_IRQ
>>> and call_do_irq :) *Grabs popcorn*
>
> All that %% is blinding, yes.
>
> Inline tabs are bad taste.
>
> Operand names instead of numbers are great for obfuscation, and nothing
> else -- unless you have more than four or five operands, in which case
> you have bigger problems already.
>
> Oh, this function is a good example of proper use of local register asm,
> btw.
>
> Comments like "// Inputs" are just harmful. As is the "creative"
> indentation here. Both harm readability and do not help understanding
> in any other way either.
>
> The thing about inline asm is the smallest details change meaning of the
> whole, it is a very harsh environment, you are programming both in C and
> directly assembler code as well, and things have to be valid for both,
> although on the other hand there is almost no error checking. Keeping
> it small, simple, readable is paramount.
>
> The rules for using inline asm:
>
> 0) Do no use inline asm.
> 1) Use extended asm, unless you know all differences with basic asm, and
> you know you want that. And if you answer "yes I do" to the latter,
> you answered wrong to the former.
> 2) Do not use toplevel asm.
> 3) Do no use inline asm.
> 4) Do no use inline asm.
> 5) Do no use inline asm.
>
> Inline asm is a very powerful escape hatch. Like all emergency exits,
> you should not use them if you do not need them! :-)
>
> But, you are talking about the function calling and the frame change I
> bet :-) Both of these are only okay because everything is back as it
> was when this (single!) asm is done, and the state created is perfectly
> fine (this is very dependent on exact ABI used, etc.)
>
> I would have used real assembler code here (in a .s file). But there
> probably are reasons to do things this way, performance probably?
We changed it to inline asm in order to ... inline it in the caller.
I also find that those operand names make it awull more difficult to
read that traditional numbering. I really dislike that new trend.
And same with those // comments, better use meaningfull C variable names.
Christophe
