Re: [RFC PATCH 04/11] drm/bridge: ti-sn65dsi86: Use bitmask to store valid rates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Doug,

On Tue, Mar 23, 2021 at 10:10 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
> On Sun, Mar 21, 2021 at 8:02 PM Laurent Pinchart
> <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> wrote:
> >
> > The valid rates are stored in an array of 8 booleans. Replace it with a
> > bitmask to save space.
>
> I'm curious: do you have evidence that this does anything useful? I
> guess you're expecting it to save .text space, right? Stack usage and
> execution time differences should be irrelevant--it's not in a
> critical section and the difference should be tiny anyway. As far as
> .text segment goes, it's not obvious to me that the compiler will use
> fewer instructions to manipulate bits compared to booleans.
>
> Doing a super simple "ls -ah" on vmlinux (unstripped):
>
> Before: 224820232 bytes
> After: 224820376 bytes
>
> ...so your change made it _bigger_.   OK, so running "strip
> --strip-debug" on those:
>
> Before: 26599464 bytes
> After: 26599464 bytes

I've been surprised by the counter-intuitive impact of similar changes
before, too.  The result may also differ a lot between arm32 or arm64.

> ...so exactly the same. I tried finding some evidence using "readelf -ah":
>
> Before:
>   [ 2] .text             PROGBITS         ffffffc010010000  00020000
>        0000000000b03508  0000000000000000 WAX       0     0     65536
>   [ 3] .rodata           PROGBITS         ffffffc010b20000  00b30000
>        00000000002e84b3  0000000000000000 WAMS       0     0     4096
>
> After:
>   [ 2] .text             PROGBITS         ffffffc010010000  00020000
>        0000000000b03508  0000000000000000 WAX       0     0     65536
>   [ 3] .rodata           PROGBITS         ffffffc010b20000  00b30000
>        00000000002e84b3  0000000000000000 WAMS       0     0     4096
>
> Maybe you have some evidence showing an improvement? Ah, OK. I
> disassembled ti_sn_bridge_enable() and your patch saves 12 bytes, but
> I guess maybe alignment washes it out in reality...

Yes, arm64 is bad w.r.t. this.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux