Re: [PATCH] FDDI: defxx: simplify if-if to if-else

[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