Small optimizations

[jeanlemotan]

• Optimized redundant dereferencing of the state
• Cached the register for FIQ_WRITE and FIQ_READ
• collapsed come redundant branches and added early outs where possible

[popcornmix]

@P33M for review.

[P33M]

Have you looked at the assembler output for the functions before and after your "optimisations"?

GCC will perform the computation of an offset once if the same offset is referenced multiple times in a function call. This applies to both register offsets and struct offsets. I suspect that GCC is already doing automatically what you are doing manually.

Likewise in fiq_fsm_tt_in_use() - is there any evidence in the emitted code that the fast return is any quicker in terms of number of instructions than the subsequent value test and break?

Your choice of hw register name causes the code to be less readable - "reg + offset"? Shouldn't that be a "channel_base + offset"?

[jeanlemotan]

You're right, reg_base would be clearer than "reg" as some offset is always added.
I didn't look at the generated assembler but I did notice the object file got significantly smaller. No numbers though as I did this change some weeks ago. And of course that doesn't tell much so I'll try to check the asm tonight.

The early return in fiq_fsm_tt_in_use is mostly for clarity IMO. The code was essentially breaking the loop to return if the channel was used and I think the intention is clearer now. Collapsing the branch (st->channel[i].fsm == FIQ_PASSTHROUGH) into the switch statement is probably irrelevant in terms of speed but again - I think it streamlines the function a bit. All the decisions based on the fsm state are taken in the switch.

[jeanlemotan]

Here's the asm for fiq_fsm_tt_in_use:

Before the commit:
0xc0255ad8 <+0>: add r3, r2, r2, lsl #4
0xc0255adc <+4>: cmp r1, #0
0xc0255ae0 <+8>: add r3, r0, r3, lsl #2
0xc0255ae4 <+12>: push {r4, r5, r6, lr}
0xc0255ae8 <+16>: ldr r5, [r3, #68] ; 0x44
0xc0255aec <+20>: ldr r6, [r3, #72] ; 0x48
0xc0255af0 <+24>: ble 0xc0255b44 <fiq_fsm_tt_in_use+108>
0xc0255af4 <+28>: mov r3, #0
0xc0255af8 <+32>: mov r4, #1
0xc0255afc <+36>: cmp r3, r2
0xc0255b00 <+40>: add r3, r3, #1
0xc0255b04 <+44>: beq 0xc0255b38 <fiq_fsm_tt_in_use+96>
0xc0255b08 <+48>: ldr r12, [r0, #60] ; 0x3c
0xc0255b0c <+52>: cmp r12, #0
0xc0255b10 <+56>: sub lr, r12, #9
0xc0255b14 <+60>: beq 0xc0255b38 <fiq_fsm_tt_in_use+96>
0xc0255b18 <+64>: cmp lr, #6
0xc0255b1c <+68>: lsl r12, r4, lr
0xc0255b20 <+72>: bhi 0xc0255b38 <fiq_fsm_tt_in_use+96>
0xc0255b24 <+76>: tst r12, #121 ; 0x79
0xc0255b28 <+80>: beq 0xc0255b38 <fiq_fsm_tt_in_use+96>
0xc0255b2c <+84>: ldr r12, [r0, #68] ; 0x44
0xc0255b30 <+88>: cmp r5, r12
0xc0255b34 <+92>: beq 0xc0255b4c <fiq_fsm_tt_in_use+116>
0xc0255b38 <+96>: cmp r3, r1
0xc0255b3c <+100>: add r0, r0, #68 ; 0x44
0xc0255b40 <+104>: bne 0xc0255afc <fiq_fsm_tt_in_use+36>
0xc0255b44 <+108>: mov r0, #0
0xc0255b48 <+112>: pop {r4, r5, r6, pc}
0xc0255b4c <+116>: ldr r12, [r0, #72] ; 0x48
0xc0255b50 <+120>: cmp r6, r12
0xc0255b54 <+124>: bne 0xc0255b38 <fiq_fsm_tt_in_use+96>
0xc0255b58 <+128>: mov r0, #1
0xc0255b5c <+132>: pop {r4, r5, r6, pc}
End of assembler dump.

After:
0xc0255ad8 <+0>: add r3, r2, r2, lsl #4
0xc0255adc <+4>: cmp r1, #0
0xc0255ae0 <+8>: add r3, r0, r3, lsl #2
0xc0255ae4 <+12>: push {r4, r5, lr}
0xc0255ae8 <+16>: ldr r4, [r3, #68] ; 0x44
0xc0255aec <+20>: ldr r5, [r3, #72] ; 0x48
0xc0255af0 <+24>: ble 0xc0255b50 <fiq_fsm_tt_in_use+120>
0xc0255af4 <+28>: mov r3, #0
0xc0255af8 <+32>: b 0xc0255b14 <fiq_fsm_tt_in_use+60>
0xc0255afc <+36>: bcc 0xc0255b08 <fiq_fsm_tt_in_use+48>
0xc0255b00 <+40>: cmp lr, #3
0xc0255b04 <+44>: bls 0xc0255b30 <fiq_fsm_tt_in_use+88>
0xc0255b08 <+48>: cmp r3, r1
0xc0255b0c <+52>: add r0, r0, #68 ; 0x44
0xc0255b10 <+56>: beq 0xc0255b50 <fiq_fsm_tt_in_use+120>
0xc0255b14 <+60>: cmp r3, r2
0xc0255b18 <+64>: add r3, r3, #1
0xc0255b1c <+68>: beq 0xc0255b08 <fiq_fsm_tt_in_use+48>
0xc0255b20 <+72>: ldr r12, [r0, #60] ; 0x3c
0xc0255b24 <+76>: cmp r12, #9
0xc0255b28 <+80>: sub lr, r12, #12
0xc0255b2c <+84>: bne 0xc0255afc <fiq_fsm_tt_in_use+36>
0xc0255b30 <+88>: ldr r12, [r0, #68] ; 0x44
0xc0255b34 <+92>: cmp r4, r12
0xc0255b38 <+96>: bne 0xc0255b08 <fiq_fsm_tt_in_use+48>
0xc0255b3c <+100>: ldr r12, [r0, #72] ; 0x48
0xc0255b40 <+104>: cmp r5, r12
0xc0255b44 <+108>: bne 0xc0255b08 <fiq_fsm_tt_in_use+48>
0xc0255b48 <+112>: mov r0, #1
0xc0255b4c <+116>: pop {r4, r5, pc}
0xc0255b50 <+120>: mov r0, #0
0xc0255b54 <+124>: pop {r4, r5, pc}
End of assembler dump.

The difference is not significant - only 2 instruction. It looks that GCC does better register allocation. I expected a more dramatic difference tbh.

So I would discard the register caching and the st->channel[n] change as it's very big and with insignificant impact.
I can prepare a new commit with just the early outs if needed.

[jeanlemotan]

I'm closing this. I will make a new pull request for the cosmetic changes later on.