From: Damien Miller <djm@mindrot.org>
Subject: Re: cc seems to optimize out com_read_reg
To: Geoff Steckel <gwes@oat.com>
Cc: tech <tech@openbsd.org>
Date: Thu, 6 Nov 2025 14:58:25 +1100

On Wed, 5 Nov 2025, Geoff Steckel wrote:

> Working from OPENBSD_7_8
> Attaching acpi uart UAR1 from an AMD Ryzen 5 CPU
> BIOS ASROCK B550 PHantom Gaming 4 / American Megatrends rev 0x50011
> Highly cut down - full dmesg, etc on request
> I -think- this occurs for all amd64
> 
> in dev/ic/com.c line 1603 com_fifo_probe line 34 (+ or -)
> this: (release version 1.180)
>         for (len = 0; len < 256; len++) {
>                 timo = 2000;
>                 while (!ISSET(com_read_reg(sc, com_lsr),
>                     LSR_RXRDY) && --timo)
>                         delay(1);
>                 if (!timo || com_read_reg(sc, com_data) != (len + 1))
>                         break;
>         }
> results bad len = 0
> 
> this: (test)
>         uint8_t c = 0x5A;
>         for (len = 0; len < 256; len++) {
>                 timo = 2000;
>                 while (!ISSET(com_read_reg(sc, com_lsr),
>                     LSR_RXRDY) && --timo)
>                         delay(1);
>                 if (!timo || (c = com_read_reg(sc, com_data)) != (len + 1))
>                         break;
>         }
>         printf("fifo probe stopped at %d with %02x\n", len, c);
> results good len = 16
> 
> It looks like cc is optimizing out the com_read_reg(sc, com_data)
> unless the value is used later.
> 
> The printf is ugly. I suspect it wouldn't be approved.
> Making com_read_reg volatile didn't fix it.
> bus.h definition of bus_space_read doesn't look like it uses volatile

yikes.

Maybe the fix should be at the bus_space_read_* level, as any such
read may have side effects (e.g. in other drivers) and therefore
shouldn't be optimised away.

-d