Poor code generation for conditional statements

Go To Last Post
67 posts / 0 new

Pages

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ChaunceyGardiner wrote:
Some people are hopefully productive using C - that's what they use - and a handful claim to be productive using assembly. So be it, that's their choice. On a platform as simple as an AVR it may even be true that they are productive, and their code is hopefully easy to understand for others that know the language (and the hardware).
That sounds like an "either or". You forgot the "both". Often speed is really of concern in only small parts of a program (e.g. a ISR). Writing that small part in assembly is not a maintaining nightmare.

ChaunceyGardiner wrote:
When you find ways to improve the code you generate, using the language you use.
The downside is that you have to reinvestigate these improvements with every new compiler version. With a small piece of assembler code you ensure to get the best result "now and forever".

And thats the point the people here are trying to make: most of the time the speed is not really of concern, then write it in C in a way that is most readable for you, not for the compiler. In the very few other cases use a small piece of assembler.

Stefan Ernst

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
No compiler is intelligent enough
It's just not how compilers currently work.

That is why I inspect the generated code.

clawson wrote:
However I still think your thread is valid simply to say "beware and check code generation if you are concerned about size/speed". That'll always be generally true and is of more concern to resource limited embedded programmers than in any other environment.

Thank you.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:
My guess - and it's just a wild guess - is that this will yield incorrect results when used on signed integers. If a is negative the if statement will evaluate as true.

How would you expect a right shift to work for a negative signed number ?

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sternst wrote:
ChaunceyGardiner wrote:
a handful claim to be productive using assembly. So be it, that's their choice. On a platform as simple as an AVR it may even be true that they are productive, and their code is hopefully easy to understand for others that know the language (and the hardware).
That sounds like an "either or".

That's what I was responding to (with regard to my comments about assembly language).

sternst wrote:
Often speed is really of concern in only small parts of a program (e.g. a ISR). Writing that small part in assembly is not a maintaining nightmare.

I agree, although I don't seem to come across any real need for it very often.

sternst wrote:
ChaunceyGardiner wrote:
When you find ways to improve the code you generate, using the language you use.
The downside is that you have to reinvestigate these improvements with every new compiler version. With a small piece of assembler code you ensure to get the best result "now and forever".

I agree that that is a possible downside, but 1) resorting to assembly for every conditional statement is not an option and 2) we didn't care about this size problem to begin with so why does it matter more if in the future it perhaps changes back to the problem it definitely is now ?

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Cliff is the only person who has posted the results of Mr Gardiner's test code. 20, 26, 12, 26 bytes for each method.

Personally, I would not be unhappy with any of those translations. Method (3) took particular advantage of a special enum { set }.

I would prefer a better test suite. And yes, it is incredibly important to specify any test conditions.

I am not surprised that the translations were not identical. Perhaps one day, compiler writers may achieve perfection.

I still maintain that: "use straightforward code, use appropriate variable types" is as good advice as any.

Note that uint8_t would have made a significant difference. However this may not be your natural style. (feeling comfortable is important for your well-being)

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:
Quote:

Huh! Can you please tell us more?

JW

My guess - and it's just a wild guess - is that this will yield incorrect results when used on signed integers. If a is negative the if statement will evaluate as true.

- Dean :twisted:

There is no incorrect result in that case:
C99, 6.5.7#5 wrote:
The result of E1 >> E2 is E1 right-shifted E2 bit positions.[...] If E1 has a signed type and a negative value, the resulting value is implementation-defined.
JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Cliff is the only person who has posted the results of Mr Gardiner's test code.

ChaunceyGardiner wrote:
2) costs 2 bytes more than 1)
4) costs 12 bytes more than 3)

david.prentice wrote:
20, 26, 12, 26 bytes for each method.

The differences of those numbers are bigger than the ones I find.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
abcminiuser wrote:
Quote:

Huh! Can you please tell us more?

JW

My guess - and it's just a wild guess - is that this will yield incorrect results when used on signed integers. If a is negative the if statement will evaluate as true.

- Dean :twisted:

There is no incorrect result in that case:
C99, 6.5.7#5 wrote:
The result of E1 >> E2 is E1 right-shifted E2 bit positions.[...] If E1 has a signed type and a negative value, the resulting value is implementation-defined.
JW

Indeed, I meant bug as in "bug in the user code due to incorrect assumptions", rather than "bug in the compiler itself". If SprinterSB was talking about compiler bugs then all bets are off, I was just thinking of the puzzle from the "what would the user do wrong" perspective.

- Dean :twisted:

Make Atmel Studio better with my free extensions. Open source and feedback welcome!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
I still maintain that: "use straightforward code, use appropriate variable types" is as good advice as any.

Note that uint8_t would have made a significant difference. However this may not be your natural style. (feeling comfortable is important for your well-being)

"Feeling comfortable" may be the key in cases like these, indeed.

Sid is apparently not comfortable with the way the compiler performs. Nor is he comfortable with writing asm either. Nor he is not comfortable with making it easy for the gcc developers to understand his complains (which would increase the probability that the compiler could be improved one day).

On the other hand, he appears to be comfortable with inspection of the generated code and tweaking the source to get a desired result, and he appears to be comfortable with all the gotchas this method involves.

Programming is all about tradeoffs. I personally wouldn't recommend Sid's preferences, though.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Those differences are bigger than the ones I find.

Yes. But at least Cliff posted the results.
You just claim things without showing any evidence.

Yes. I may prefer one style which I have found works pretty well across many platforms.

You have to make your own choice of style.

Incidentally, I have altered 'styles' to squeeze into a bootloader section. In all honesty, I have never bothered with an application section. How often are you 20 bytes too big?

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

wek wrote:
he appears to be comfortable with inspection of the generated code and tweaking the source to get a desired result, and he appears to be comfortable with all the gotchas this method involves.

What exact gotchas are we talking about here ? That a future compiler will generate the opposite relationship with regards to size that we are seeing now ?

Somehow it is better to accept a definite penalty now in case it turns out to be an advantage with a hypothetical future reversal of problems in the compiler ?

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

abcminiuser wrote:
If SprinterSB was talking about compiler bugs [...]
As far as I know him, he does not acknowledge user errors as "bugs"... ;-)

I tried to play a bit with the >>7, and although did not find anything suspicious, the following in context of Cliff's test might be some food for thought:

#include 

volatile uint8_t a;
volatile uint8_t b;

int main(void) {
  if (a >> 7) {
    b = 1;
  } else {
    b = 2;
  }

  b = (a >> 7) ? 3 : 4;

}
int main(void) {
  if (a >> 7) {
 112:	80 91 01 02 	lds	r24, 0x0201
 116:	87 ff       	sbrs	r24, 7
 118:	02 c0       	rjmp	.+4      	; 0x11e 
    b = 1;
 11a:	81 e0       	ldi	r24, 0x01	; 1
 11c:	01 c0       	rjmp	.+2      	; 0x120 
  } else {
    b = 2;
 11e:	82 e0       	ldi	r24, 0x02	; 2
 120:	80 93 00 02 	sts	0x0200, r24
  }

  b = (a >> 7) ? 3 : 4;
 124:	80 91 01 02 	lds	r24, 0x0201
 128:	80 95       	com	r24
 12a:	88 1f       	adc	r24, r24
 12c:	88 27       	eor	r24, r24
 12e:	88 1f       	adc	r24, r24
 130:	8d 5f       	subi	r24, 0xFD	; 253
 132:	80 93 00 02 	sts	0x0200, r24
 136:	08 95       	ret

00000138 <_exit>:
 138:	ff cf       	rjmp	.-2      	; 0x138 <_exit>

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Quote:
Those differences are bigger than the ones I find.

Yes. But at least Cliff posted the results.
You just claim things without showing any evidence.


Huh ? I posted the relevant numbers and I posted the code with instructions about where to put it if you wanted to verify those numbers.

The numbers you seem to like are clearly wrong, too.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oops. I did my sums wrong on the byte counts:
22, 28, 12, 28 bytes.

Quote:

Huh ? I posted the relevant numbers and I posted the code with instructions about where to put it if you wanted to verify those numbers.

Is there any chance of you posting the generated code?

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Is there any chance of you posting the generated code?

Why ? What possible benefit could there be ?

david.prentice wrote:
How often are you 20 bytes too big?

I frequently have more than one conditional statement in my programs.

Sid

Life... is a state of mind

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Please post a bug report at the GCC bug database here:
http://gcc.gnu.org/bugzilla/

This page at the GCC project goes into detail as to what is needed in a bug report:
http://gcc.gnu.org/bugs/

Please put my email address (see button at the bottom of my posts) in the CC field.

Note that if there is no compilable test case that shows the misoptimization then the bug cannot be fixed.

We want to make sure that misoptimization bugs get fixed.

Pages