From: Maciej W. Rozycki > Sent: 03 March 2022 09:55 > > > > 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 > > (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. Or just not worry that the 'char' value (either [128..127] or [0..255]) is held in a 'signed int' variable. That basically happens every time it is loaded into a register anyway. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
