Re: [PATCH] FDDI: defxx: simplify if-if to if-else
From: Jiabing Wan
Date: Sun Apr 24 2022 - 23:28:10 EST
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