Thread: Sample Function Optimisations For a Novice

My first tip would be to rewrite it so you don't have any ?: operators in the code.
The code as is is basically unreadable.

For example,

Code:

uint16_t myDuty = myValue? (uint16_t) (myValue*100U) / halfPeriod : 0U;

Becomes

Code:

uint16_t myDuty;
if ( myValue ) {
    myDuty = (uint16_t) (myValue*100U) / halfPeriod;
} else {
    myDuty = 0;
}

> uint16_t __getLedDuty
To quote the C99 standard.

7.1.3 Reserved identifiers
All identifiers that begin with an underscore and either an uppercase letter or another
underscore are always reserved for any use.

Originally Posted by snoop33

That's a style over optimisation though, I prefer the ?:; shorthand.

It is true that it is a matter of style, but nesting ternary operators is pretty much always poor style, so at the very least you should get rid of that.

Originally Posted by snoopy33

Is there a means in C to say a function should only be available for it's .c file? (aka private)?

Declare it static. Of course, this means you should not declare it in a header.

> That's a style over optimisation though
Optimisation has very little to do with the number of characters in your source code.

Code:

#include <stdio.h>

typedef unsigned short uint16_t;

uint16_t foo ( uint16_t myValue, uint16_t halfPeriod ) {
    uint16_t myDuty = myValue? (uint16_t) (myValue*100U) / halfPeriod : 0U;
    return myDuty;
}

uint16_t bar ( uint16_t myValue, uint16_t halfPeriod ) {
    uint16_t myDuty;
    if ( myValue ) {
        myDuty = (uint16_t) (myValue*100U) / halfPeriod;
    } else {
        myDuty = 0;
    }
    return myDuty;
}

The respective assembler codes with no optimisation at all.

Code:

foo:
	pushq	%rbp
	movq	%rsp, %rbp
	movl	%edi, %edx
	movl	%esi, %eax
	movw	%dx, -20(%rbp)
	movw	%ax, -24(%rbp)
	cmpw	$0, -20(%rbp)
	je	.L2
	movzwl	-20(%rbp), %edx
	movl	%edx, %eax
	sall	$2, %eax
	addl	%edx, %eax
	leal	0(,%rax,4), %edx
	addl	%edx, %eax
	sall	$2, %eax
	movl	$0, %edx
	divw	-24(%rbp)
	jmp	.L3
.L2:
	movl	$0, %eax
.L3:
	movw	%ax, -2(%rbp)
	movzwl	-2(%rbp), %eax
	popq	%rbp
	ret

bar:
	pushq	%rbp
	movq	%rsp, %rbp
	movl	%edi, %edx
	movl	%esi, %eax
	movw	%dx, -20(%rbp)
	movw	%ax, -24(%rbp)
	cmpw	$0, -20(%rbp)
	je	.L6
	movzwl	-20(%rbp), %edx
	movl	%edx, %eax
	sall	$2, %eax
	addl	%edx, %eax
	leal	0(,%rax,4), %edx
	addl	%edx, %eax
	sall	$2, %eax
	movl	$0, %edx
	divw	-24(%rbp)
	movw	%ax, -2(%rbp)
	jmp	.L7
.L6:
	movw	$0, -2(%rbp)
.L7:
	movzwl	-2(%rbp), %eax
	popq	%rbp
	ret

Compile with optimisation, and you get less instructions. But still the SAME set of instructions.

The rule nowadays is go for readability every time unless you have a damn good reason and lots of evidence (not 30 year old heresay) that cryptic compressed code is actually better. Mostly it won't be, because code that confuses human readers also confuses code optimisers.

Speaking of that, I note that if myValue == 0, the computation would still result in myDuty == 0. Considering the computation is just a multiplication, a cast, and a division, it looks unnecessary to treat it as a special case unless you're saying that a non-zero myValue is expected to be rare. If not, this is premature optimisation (and might even be a "pessimisation" if it turns out that myValue == 0 is rare).