On 11/12/2020 07:34:41+0100, Markus Elfring wrote:
> >> How do you think about a patch like “staging: speakup: remove redundant
> >> initialization
> >> of pointer p_key” for comparison?
> >> https://lore.kernel.org/patchwork/patch/1199128/
> >> https://lore.kernel.org/driverdev-devel/20200223153954.420731-1-colin.k...@canonical.com/
> >>
> >> Would you tolerate to omit the initialisation for the variable “slot”?
> >
> > If you were able to provide one good technical reason.
>
> I find that the positions of variable definitions (and similar assignments)
> influence
> the generation of executable code.
>
And you are wrong, it doesn't. Before:
c044a0f0 <atmci_request_end>:
{
c044a0f0: e92d4070 push {r4, r5, r6, lr}
c044a0f4: e1a04000 mov r4, r0
WARN_ON(host->cmd || host->data);
c044a0f8: e5902024 ldr r2, [r0, #36] ; 0x24
{
c044a0fc: e1a06001 mov r6, r1
struct mmc_host *prev_mmc = host->cur_slot->mmc;
c044a100: e590301c ldr r3, [r0, #28]
WARN_ON(host->cmd || host->data);
c044a104: e3520000 cmp r2, #0
struct mmc_host *prev_mmc = host->cur_slot->mmc;
c044a108: e5935000 ldr r5, [r3]
WARN_ON(host->cmd || host->data);
c044a10c: 0a00002d beq c044a1c8 <atmci_request_end+0xd8>
c044a110: e3000000 movw r0, #0
c044a110: R_ARM_MOVW_ABS_NC .LC0
c044a114: e3a03000 mov r3, #0
c044a118: e3400000 movt r0, #0
c044a118: R_ARM_MOVT_ABS .LC0
c044a11c: e3a02009 mov r2, #9
c044a120: e300161c movw r1, #1564 ; 0x61c
c044a124: ebfffffe bl 0 <warn_slowpath_fmt>
c044a124: R_ARM_CALL warn_slowpath_fmt
del_timer(&host->timer);
c044a128: e28400a4 add r0, r4, #164 ; 0xa4
c044a12c: ebfffffe bl 0 <del_timer>
c044a12c: R_ARM_CALL del_timer
if (host->need_clock_update) {
c044a130: e5d430a0 ldrb r3, [r4, #160] ; 0xa0
c044a134: e3530000 cmp r3, #0
c044a138: 0a000005 beq c044a154 <atmci_request_end+0x64>
atmci_writel(host, ATMCI_MR, host->mode_reg);
c044a13c: e59420b8 ldr r2, [r4, #184] ; 0xb8
c044a140: e5943000 ldr r3, [r4]
asm volatile("str %1, %0"
c044a144: e5832004 str r2, [r3, #4]
if (host->caps.has_cfg_reg)
c044a148: e5d420da ldrb r2, [r4, #218] ; 0xda
c044a14c: e3520000 cmp r2, #0
c044a150: 1a000019 bne c044a1bc <atmci_request_end+0xcc>
host->cur_slot->mrq = NULL;
c044a154: e594101c ldr r1, [r4, #28]
return READ_ONCE(head->next) == head;
c044a158: e1a03004 mov r3, r4
c044a15c: e3a02000 mov r2, #0
c044a160: e5812010 str r2, [r1, #16]
host->mrq = NULL;
c044a164: e5842020 str r2, [r4, #32]
c044a168: e5b31098 ldr r1, [r3, #152]! ; 0x98
if (!list_empty(&host->queue)) {
c044a16c: e1510003 cmp r1, r3
host->state = STATE_IDLE;
c044a170: 05842094 streq r2, [r4, #148] ; 0x94
if (!list_empty(&host->queue)) {
c044a174: 0a00000c beq c044a1ac <atmci_request_end+0xbc>
slot = list_entry(host->queue.next,
c044a178: e5943098 ldr r3, [r4, #152] ; 0x98
After:
c044a0f0 <atmci_request_end>:
{
c044a0f0: e92d4070 push {r4, r5, r6, lr}
c044a0f4: e1a04000 mov r4, r0
WARN_ON(host->cmd || host->data);
c044a0f8: e5902024 ldr r2, [r0, #36] ; 0x24
{
c044a0fc: e1a06001 mov r6, r1
struct mmc_host *prev_mmc = host->cur_slot->mmc;
c044a100: e590301c ldr r3, [r0, #28]
WARN_ON(host->cmd || host->data);
c044a104: e3520000 cmp r2, #0
struct mmc_host *prev_mmc = host->cur_slot->mmc;
c044a108: e5935000 ldr r5, [r3]
WARN_ON(host->cmd || host->data);
c044a10c: 0a00002d beq c044a1c8 <atmci_request_end+0xd8>
c044a110: e3000000 movw r0, #0
c044a110: R_ARM_MOVW_ABS_NC .LC0
c044a114: e3a03000 mov r3, #0
c044a118: e3400000 movt r0, #0
c044a118: R_ARM_MOVT_ABS .LC0
c044a11c: e3a02009 mov r2, #9
c044a120: e300161b movw r1, #1563 ; 0x61b
c044a124: ebfffffe bl 0 <warn_slowpath_fmt>
c044a124: R_ARM_CALL warn_slowpath_fmt
del_timer(&host->timer);
c044a128: e28400a4 add r0, r4, #164 ; 0xa4
c044a12c: ebfffffe bl 0 <del_timer>
c044a12c: R_ARM_CALL del_timer
if (host->need_clock_update) {
c044a130: e5d430a0 ldrb r3, [r4, #160] ; 0xa0
c044a134: e3530000 cmp r3, #0
c044a138: 0a000005 beq c044a154 <atmci_request_end+0x64>
atmci_writel(host, ATMCI_MR, host->mode_reg);
c044a13c: e59420b8 ldr r2, [r4, #184] ; 0xb8
c044a140: e5943000 ldr r3, [r4]
asm volatile("str %1, %0"
c044a144: e5832004 str r2, [r3, #4]
if (host->caps.has_cfg_reg)
c044a148: e5d420da ldrb r2, [r4, #218] ; 0xda
c044a14c: e3520000 cmp r2, #0
c044a150: 1a000019 bne c044a1bc <atmci_request_end+0xcc>
host->cur_slot->mrq = NULL;
c044a154: e594101c ldr r1, [r4, #28]
return READ_ONCE(head->next) == head;
c044a158: e1a03004 mov r3, r4
c044a15c: e3a02000 mov r2, #0
c044a160: e5812010 str r2, [r1, #16]
host->mrq = NULL;
c044a164: e5842020 str r2, [r4, #32]
c044a168: e5b31098 ldr r1, [r3, #152]! ; 0x98
if (!list_empty(&host->queue)) {
c044a16c: e1510003 cmp r1, r3
host->state = STATE_IDLE;
c044a170: 05842094 streq r2, [r4, #148] ; 0x94
if (!list_empty(&host->queue)) {
c044a174: 0a00000c beq c044a1ac <atmci_request_end+0xbc>
struct atmel_mci_slot *slot = list_entry(host->queue.next,
c044a178: e5943098 ldr r3, [r4, #152] ; 0x98
Do you realize your patch is just unnecessary churn now?
Is it too difficult for you to actually compile the driver and look
at the changes before submitting patches?
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
