Messages in this thread |  | | Date | Mon, 25 Apr 2022 11:27:46 +0800 | Subject | Re: [PATCH] FDDI: defxx: simplify if-if to if-else | From | Jiabing Wan <> |
| |
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
|  |