Hi Maciej,
Le jeu., mars 3 2022 at 09:55:17 +0000, Maciej W. Rozycki
<macro@xxxxxxxxxxx> a écrit :
On Thu, 3 Mar 2022, Jiri Slaby wrote:
> The real problem is that using char (or short) for a function
parameter
> or result is very likely to require the compile add code to mask
> the value to 8 (or 16) bits.
>
> Remember that almost every time you do anything with a signed or
unsigned
> char/short variable the compiler has to use the integer promotion
rules
> to convert the value to int.
>
> You'll almost certainly get better code if the value is left in an
> int (or unsigned int) variable until the low 8 bits get written to
> a buffer (or hardware register).
So should we use int/uint instead of more appropriate shorter types
everywhere
now? The answer is: definitely not. The assembly on x86 looks good
(it uses
movz, no ands), RISC architectures have to do what they chose to.
We do have an issue, because we still have this:
void uart_console_write(struct uart_port *port, const char *s,
unsigned int count,
void (*putchar)(struct uart_port *, int))
and then:
putchar(port, *s);
there. Consequently on targets where plain `char' type is signed the
value retrieved from `*s' has to be truncated in the call to
`putchar'.
And indeed it happens with the MIPS target:
803ae47c: 82050000 lb a1,0(s0)
803ae480: 26100001 addiu s0,s0,1
803ae484: 02402025 move a0,s2
803ae488: 0220f809 jalr s1
803ae48c: 30a500ff andi a1,a1,0xff
vs current code:
803ae47c: 82050000 lb a1,0(s0)
803ae480: 26100001 addiu s0,s0,1
803ae484: 0220f809 jalr s1
803ae488: 02402025 move a0,s2
And how is that at all a problem?
(NB the last instruction shown after the call instruction, JALR, is
in the
delay slot that is executed before the PC gets updated). Now
arguably the
compiler might notice that and use an unsigned LBU load instruction
rather
than the signed LB load instruction, which would make the ANDI
instruction
redundant, but still I think we ought to avoid gratuitous type
signedness
changes.
So I'd recommend changing `s' here to `const unsigned char *' or, as
I
previously suggested, maybe to `const u8 *' even.
Just cast the string to "const u8 *" within the function, while keeping
a "const char *s" argument. The compiler will then most likely generate
LBUs.
Cheers,
-Paul
