Hi Doug,
On Tue, Mar 23, 2021 at 10:10 PM Doug Anderson dianders@chromium.org wrote:
On Sun, Mar 21, 2021 at 8:02 PM Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com 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