From: Jiabing Wan <wanjiabing@vivo.com>
To: "Maciej W. Rozycki" <macro@orcam.me.uk>, Andrew Lunn <andrew@lunn.ch>
Cc: "David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kael_w@yeah.net
Subject: Re: [PATCH] FDDI: defxx: simplify if-if to if-else
Date: Mon, 25 Apr 2022 11:27:46 +0800 [thread overview]
Message-ID: <6568e274-a013-4ab0-2c6d-228014e605b5@vivo.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2204250009240.9383@angie.orcam.me.uk>
On 2022/4/25 7:26, Maciej W. Rozycki wrote:
> On Mon, 25 Apr 2022, Andrew Lunn wrote:
>
>>> NAK. The first conditional optionally sets `bp->mmio = false', which
>>> changes the value of `dfx_use_mmio' in some configurations:
>>>
>>> #if defined(CONFIG_EISA) || defined(CONFIG_PCI)
>>> #define dfx_use_mmio bp->mmio
>>> #else
>>> #define dfx_use_mmio true
>>> #endif
Yes, it's my fault. I didn't notice "dfx_use_mmio" is a MACRO,
sorry for this wrong patch.
>> It probably won't stop the robots finding this if (x) if (!x), but
>> there is a chance the robot drivers will wonder why it is upper case.
> Well, blindly relying on automation is bound to cause trouble. There has
> to be a piece of intelligence signing the results off at the end.
You are right and I'll be more careful to review the result before
submitting.
>
> And there's nothing wrong with if (x) if (!x) in the first place; any
> sane compiler will produce reasonable output from it. Don't fix what
> ain't broke! And watch out for volatiles!
Yes, there's nothing wrong with if (x) if (!x), but I want to do is
reducing the complexity of the code.
There would be less instructions when using "if and else" rather
than "if (A) and if (!A)" as I tested:
Use if(A) and if(!A):
ldr w0, [sp, 28]
cmp w0, 0
beq .L2
ldr w0, [sp, 28]
add w0, w0, 1
str w0, [sp, 28]
.L2:
ldr w0, [sp, 28] <------ one more ldr instruction
cmp w0, 0 <------ one more cmp instruction
bne .L3
ldr w0, [sp, 28]
add w0, w0, 2
str w0, [sp, 28]
.L3:
ldr w0, [sp, 28]
mov w1, w0
adrp x0, .LC1
add x0, x0, :lo12:.LC1
bl printf
Use if(A) and else:
ldr w0, [sp, 28]
cmp w0, 0
beq .L2
ldr w0, [sp, 28]
add w0, w0, 1
str w0, [sp, 28] <------ reduce two instructions
b .L3
.L2:
ldr w0, [sp, 28]
add w0, w0, 2
str w0, [sp, 28]
.L3:
ldr w0, [sp, 28]
mov w1, w0
adrp x0, .LC1
add x0, x0, :lo12:.LC1
bl printf
I also use "pmccabe" , a tool from gcc, to calculate the complexity of the code.
It shows this patch can reduce the statements in function.
Use if(A) and if(!A):
pmccabe -v test.c Modified McCabe Cyclomatic Complexity
| Traditional McCabe Cyclomatic Complexity
| | # Statements in function
| | | First line of function
| | | | # lines in function
| | | | | filename(definition line number):function
| | | | | |
3 3 8 4 17 test.c(4): main
Use if(A) and else:
pmccabe -v test.c
Modified McCabe Cyclomatic Complexity
| Traditional McCabe Cyclomatic Complexity
| | # Statements in function
| | | First line of function
| | | | # lines in function
| | | | | filename(definition line number):function
| | | | | |
2 2 7 4 16 test.c(4): main
So I think this type of patchs is meaningful.
Thanks,
Wan Jiabing
next prev parent reply other threads:[~2022-04-25 3:28 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-24 9:28 [PATCH] FDDI: defxx: simplify if-if to if-else Wan Jiabing
2022-04-24 10:39 ` Maciej W. Rozycki
2022-04-24 22:17 ` Andrew Lunn
2022-04-24 23:26 ` Maciej W. Rozycki
2022-04-25 3:27 ` Jiabing Wan [this message]
2022-04-25 12:06 ` Andrew Lunn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6568e274-a013-4ab0-2c6d-228014e605b5@vivo.com \
--to=wanjiabing@vivo.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=kael_w@yeah.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=macro@orcam.me.uk \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.