lkml.org 
[lkml]   [2022]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] FDDI: defxx: simplify if-if to if-else
From


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


\
 
 \ /
  Last update: 2022-04-25 05:30    [W:0.110 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site