__builtin_avr_insert_bits weirdness (avr-gcc)

Go To Last Post
23 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I was wondering why whenever I assign a bit to a different bit in an output register (e.g., bit 7 in a variable to bit 0 in a register) using bit fields this code is generated:

in r21,0x8
bst r20,7
bld r21,0
out 0x8,r21

but when I assign a bit to the same position (e.g., bit 0 in variable to bit 0 in register, or bit 1 to bit 1, etc) this code is generated instead (slower by one cycle, although only one register is used):

sbrc r20,0
sbi 0x8,0
sbrs r20,0
cbi 0x8,0

 

So I tried to force a bld/bst with __builtin_avr_insert_bits, and a weird thing happens. This is the code I compiled with -Os for atmega328p:

#include <avr/io.h>

char var;
int main()
{
  PORTC = __builtin_avr_insert_bits (0xfffffff7, var, PORTC);
}

When source and destination bits are different I get exactly the same as with bit fields. The code above generates:

in r24,0x8
lds r25,var
bst r25,7
bld r24,0
out 0x8,r24

But with __builtin_avr_insert_bits(0xffffffff0, var, PORTC) I get this convoluted piece of assembler:

in r24,0x8
in r25,0x8
lds r18,var
eor r24,r18
andi r24,lo8(1)
eor r24,r25
out 0x8,r24

Which takes two cycles longer and 2 instructions more. A similar snippet is generated for __builtin_avr_insert_bits(0xfffffff1f, var, PORTC), __builtin_avr_insert_bits(0xffffff2ff, var, PORTC), etc.

 

Is this a bug or is there something I'm not taking into account?

Last Edited: Wed. Dec 19, 2018 - 11:55 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Explain why this matters to you?

If time and space matters, why use a HLL instead of assembly?

How could it be a bug if it performs the correct operation?

 

Jim

 

 

 

 

Keys to wealth:

Invest for cash flow, not capital gains!

Wealth is attracted, not chased! 

Income is proportional to how many you serve!

 

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

ki0bk wrote:
Explain why this matters to you?

If time and space matters, why use a HLL instead of assembly?

Didn't said it mattered. I'm curious, and this seems to be "A forum for discussions about compilers and general programming topics".

 

ki0bk wrote:
How could it be a bug if it performs the correct operation?

Huh? If the compiler duplicated every "bst" instruction the program would do the same thing and it would be a bug. Generating unnecessary code is a compiler bug.

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

vaar wrote:
Generating unnecessary code is a compiler bug.

Is it?  Or is it just an inefficiency? 

In the example shown, if I understand your posting, the correct bit(s) get set, it just takes longer one way over the other.

Not a bug, just an inefficiency.   It is open source, contribute your better idea.

 

Jim

 

 

Keys to wealth:

Invest for cash flow, not capital gains!

Wealth is attracted, not chased! 

Income is proportional to how many you serve!

 

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

ki0bk wrote:
Not a bug, just an inefficiency.   It is open source, contribute your better idea.

It would be just an inefficiency if all cases were treated the same and I found out a more efficient way. But since the case where it occurs is a very specific one and faster+shorter code is generated for all the other cases, either

 

a) it's a bug, because the generation of the in/bst/bld/out (already programmed in avr-gcc) is wrongly avoided when the input and output bits are in the same position

b) there is a good reason to avoid it in that case (which I'm not seeing), like an undesired side effect when using the T flag

 

which is what I'm asking. "Who cares; it works" seems to me like a bad reason to avoid the more efficient (and more general) code.

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

vaar wrote:
like an undesired side effect when using the T flag

Perhaps its a desired side effect yet unkown (by you), I'm not criticizing your motivation or curiosity,  just disagreeing that it is a "bug". 

I'm just a user of tools, not a maker of them, so you may well find others here that agree with you. 

Any way, pull a copy of the code and purpose an improvement "bug fix" to those that do make the tools. 

Have fun!

 

Jim

 

 

Keys to wealth:

Invest for cash flow, not capital gains!

Wealth is attracted, not chased! 

Income is proportional to how many you serve!

 

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

Code like 

sbrc r20,0
sbi 0x8,0
sbrs r20,0
cbi 0x8,0

 

You should be careful about side effects if r20 can change in an ISR then this code is bad.

The code is actually better than some of the other code and R20 should not change in an ISR

 

And never use this structure with SBIC and SBIS for the same reason.   

Last Edited: Sat. Dec 15, 2018 - 03:07 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Generating unnecessary code is a compiler bug

Sorry, no.  Generating unnecessary code is inefficient.  If the end result is correct operation, it is not a bug.  Does the code do what it says it will do?  Yes?  Then no bug.  Does the code make any claims about executions speed, or flash usage?  Or register usage?  I don't believe it does.  Not a bug.  At worst it is a missed optimisation.  That is different, >>very different<<, from an actual bug leading to incorrect operation.  Both types of defects can be submitted to the GCC bug tracker, but they will get assigned different priorities.

 

Curiously, you may have stumbled across an actual bug, but not the one you think:

in r24,0x8
in r25,0x8
lds r18,var
eor r24,r18
andi r24,lo8(1)
eor r24,r25
out 0x8,r24

 

Note how PORTC is accessed twice.  Since PORTC is volatile, there are rules which govern when and how often the compiler is permitted to access (read or write) it, because there may be side effects when doing so.  In the case of PORTC, there are no side effects of reading, so no harm done, but that is by no means certain with other volatile I/O registers, where the act of reading can have genuine side effects on machine state.

 

There are no warnings concerning possible multiple accesses in the GCC documentation:

https://gcc.gnu.org/onlinedocs/gcc/AVR-Built-in-Functions.html

 

I'm curious to hear what others have to say about this, especially those taking part in this other thread on compiler defects:

https://www.avrfreaks.net/forum/avr-gcc-optimization-xor-incorrectly-eliminates-volatile-access

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

joeymorin wrote:

Generating unnecessary code is a compiler bug

Sorry, no.  Generating unnecessary code is inefficient.  If the end result is correct operation, it is not a bug.  Does the code do what it says it will do?  Yes?  Then no bug.  Does the code make any claims about executions speed, or flash usage?  Or register usage?  I don't believe it does.  Not a bug.  At worst it is a missed optimisation.  That is different, >>very different<<, from an actual bug leading to incorrect operation.  Both types of defects can be submitted to the GCC bug tracker, but they will get assigned different priorities.

Well, I disagree. If, as I put in my previous example, there was a bad condition in the compiler (e.g. <= instead of <) that caused every "bst" instruction to be duplicated (and only caused that) the resulting code would work without a hitch and I think everybody would agree it *is* a bug. From wikipedia: "A software bug is an error, flaw, failure or fault in a computer program or system that causes it to produce an incorrect or unexpected result, or to behave in unintended ways".

 

In this case, if ALL the bit-copy cases were treated the same, I'd say, "hey, this can be improved". But only one specific case is treated differently without seemingly any reason, which makes me doubt that it's the way developer intended his code (the compiler) to behave, and I think "hey, this should be fixed".

 

Put it another way, if you prefer: inefficient code may be the symptom of a bug, and in this case I think it is*.

 

*If (again) I'm not missing something about copying the same bit position from one place to another and the developer *did* intend his code to behave that way.

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

vaar wrote:
but when I assign a bit to the same position (e.g., bit 0 in variable to bit 0 in register, or bit 1 to bit 1, etc) this code is generated instead (slower by one cycle, although only one register is used):
vaar wrote:
Generating unnecessary code is a compiler bug.

Let's back up the sky-is-falling truck for a moment beep beep beep and explore the original assertion.

 

First, on a side note, don't you think it might be important to tell which AVR model is involved?  After all, different model series/architectures/generations might have different cycle counts.

 

Now on to the whole source of this discussion -- "slower by one cycle".  PLEASE TELL ME HOW Y'ALL REACHED THAT CONCLUSION.

so

vaar wrote:
sbrc r20,0 sbi 0x8,0 sbrs r20,0 cbi 0x8,0
  takes four cycles, one SBR or CBR for one cycle, one SBRx not taken for one cycle, one SBRx taken for two cycles.  Tell me how that is slower than the four-instruction sequence?  So, the maligned buggy compiler (your attribution) uses the same cycles and one less register...

 

Fake news?  "Never mind."  -- Emily Litella https://www.nbc.com/saturday-nig...

 

IMO y'all can keep this discussion limited to e.g. actual code sequences, and behavior of your chosen infinite-value toolchain under various conditions.  Please take your debate on what is a bug elsewhere.

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Sat. Dec 15, 2018 - 02:59 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

in #1 OP tell it's a 328P

 

the problem is that sbi and cbi take 2 clk where in and out is done in 1clk

 

In general care should be taken when <in> <modify> <out> a port is used.

If other parts of the same port are changed in an ISR , an change in an ISR there come between the in and out will be overwritten (and that is an error that is hard to find).

A <sbi> and <cbi> solution will not have that problem. 

 

 

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

that causes it to produce an incorrect or unexpected result, or to behave in unintended ways

The expectations are laid out in the documentation, as is the intended behaviour.  Neither are violated by inefficient code.  Without being rude, your personal expectations are not relevant.  An implementation could chose to implement the abstract machine using a flock of carrier pigeons with an abacus, as long as it meets the requirements of the standard.  The standard says nothing at all about efficiency.

 

I agree that your example is inefficient.  I do not agree that it is incorrect.  And it is in no way unexpected.  Nevertheless, I am not discouraging you from reporting it here, nor directly to the AVR GCC project.  However, as it is a missed optimisation and not a defect resulting in incorrect operation, do not expect it to be assigned a high priority.  As an open source project, you are welcome to submit your work to improve it.

 

You (and others, I expect) might say this is a argument about semantics, but it is after all language we are talking about.  Semantics are important.

 

I'm still curious about the possible genuine defect I noted in #8.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

theusch wrote:

....

WTF

theusch wrote:

First, on a side note, don't you think it might be important to tell which AVR model is involved?  After all, different model series/architectures/generations might have different cycle counts.

It's right there in the first post: atmega328p

theusch wrote:

Now on to the whole source of this discussion -- "slower by one cycle".  PLEASE TELL ME HOW Y'ALL REACHED THAT CONCLUSION.

sbrc/sbi/sbrs/cbi TAKES FIVE CYCLES IN ATMEGA328P (hey, I can shout too). Two for the sbrc/sbrs with true condition, one for the sbrc/sbrs with false condition, two for the sbi or cbi instruction executed.

theusch wrote:
So, the maligned buggy compiler (your attribution) uses the same cycles and one less register

Not true for the atmega328p, and that was not even the reason for my thread, just what caused me to investigate further and find out about the different snippets generated by __builtin_avr_insert_bits (which IS the reason of this thread).

 

Oh, fuck, this is getting ridiculous. Two snarky remarks that add nothing to the discussion, an offtopic debate that is going nowhere, and still no clue about the original question. To hell with this forum. Bye.

 

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

Well that didn't take long.

 

I didn't spot anywhere in this thread where you mention which version of the compiler or toolchain you're using, but for example with version 3.4.4 of the toolchain, which is version 4.8.1 of the compiler (admittedly quite old, but that's the only source I have on this computer at the moment), the code which implements __builtin_avr_insert_bits is in the compiler source file ./gcc/gcc/config/avr/avr.c.  You will need to familiarise yourself with the GCC compiler internals to make sense of that code, and how it leads to the generation of the AVR assembler instructions.

 

As I say, if you are unsatisfied with the code generation, you are free to submit patches to the project.  You could even just raise the issue in the GCC Bugzilla, but as the issue isn't to do with incorrect code generation and is at worst a missed optimisation, it likely won't be assigned a high priority.

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

In my opinion, the unexpected double read of a volatile register with possible side effects is a bug.

The bloated code that concerns the OP so much is a missed optimization... if I ranted here each time I found something like that, I'd probably been kicked of the forum by nowcheeky

 

edit: The first thread I ever created here was about missed optimizations, actually: https://www.avrfreaks.net/forum/...

I learned a lot about obscure gcc optimization options there.

Last Edited: Sun. Dec 16, 2018 - 05:31 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

vaar wrote:
To hell with this forum. Bye.

Oops.  Yes, I missed the model mention in the end of the line in the middle of a multi-screenful post.  I actually never got that far, after reading the original problem exposition.  And didn't do a very good job at that either, given that...

 

... I misread the code snippet to be SBR/CBR versus SBI/CBI.  Of course that makes a difference in cycle counts.

 

Then, to keep the ball rolling, my "snarks" were interpreted as being at OP, when really I was targeting "y'all".

 

[Does the code generation model of the toolchain and version used use the T flag for some purposes?  If so, then without save/restore it wouldn't be available for this purpose.]

 

Yes, conditional-set of an I/O bit is not a painless AVR8 operation.  There was detailed on this in th past.  Was the "best" always constant cycles, something like a double-test and the a conditional SBI to the PIN register?

 

"Never mind."  -- Emily Litella

 

 

 

 

 

 

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

theusch wrote:
onditional-set of an I/O bit is not a painless AVR8 operation. There was detailed on this in th past. Was the "best" always constant cycles, something like a double-test and the a conditional SBI to the PIN register?

 

theusch, with all due respect but I think your way of communicating is also discouraging other new members from sharing their ideas. the way you speak is somehow could be unacceptable. I remember the first time I posted something and you came inside posting a picture that obviously shows that you didnt even bother to know what the discussion is about.

 

Now because of you we lost a person who might have add to this forum and who have interesting points. clearly you dont care...but i hope you could consider that in the future. I apologize to vaar and would like to see him again in the forum.

 

Regards,

Moe

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

Moe123 wrote:
I remember the first time I posted something and you came inside posting a picture that obviously shows that you didnt even bother to know what the discussion is about. Now because of you we lost a person who might have add to this forum and who have interesting points. clearly you dont care...but i hope you could consider that in the future. I apologize to vaar and would like to see him again in the forum.

Ask the moderators to have me banned.  It has been done before.

 

Yes, I misread the OP here.  I'll give up programming microcontrollers.  [I retired a month ago]

 

Moe123 wrote:
I remember the first time I posted something and you came inside posting a picture

The first time you posted something: https://www.avrfreaks.net/commen...

I didn't participate in that thread.  Fake news?  But >>I<< am the surly curmudgeon.

 

In fact, I cannot see where I participated in any of the treads you have initiated.  But all surly curmudgeons look alike.  Perhaps you are mistaking me for another?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Tue. Dec 18, 2018 - 10:45 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:

Moe123 wrote:

I remember the first time I posted something and you came inside posting a picture that obviously shows that you didnt even bother to know what the discussion is about. Now because of you we lost a person who might have add to this forum and who have interesting points. clearly you dont care...but i hope you could consider that in the future. I apologize to vaar and would like to see him again in the forum.

 

Ask the moderators to have me banned.  It has been done before.

 

Yes, I misread the OP here.  I'll give up programming microcontrollers.  [I retired a month ago]

 

 

Moe123 wrote:

I remember the first time I posted something and you came inside posting a picture

 

The first time you posted something: https://www.avrfreaks.net/commen...

I didn't participate in that thread.  Fake news?  But >>I<< am the surly curmudgeon.

 

In fact, I cannot see where I participated in any of the treads you have initiated.  But all surly curmudgeons look alike.  Perhaps you are mistaking me for another?

 

Well, here you go https://www.avrfreaks.net/forum/....

 

theusch, I will ask nobody to ban you, nor do i intend to do so in the future, I am just asking you on behalf of many people here to respect the others, to let them share their ideas...discussions..whatever. So we can all as members of this forum enjoy and learn more, solve our technical problems...etc. nothing personal here. You are a member here since 2001, I am just a 3 or 4 months member, you have a big experience there is no doubt about this, but so the others.

 

Warm regards,

Moe

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

Well hopefully that'll be the last we'll see of this idiot but I'll ban the account just to be sure.

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

I shouldn't be feeding this, but...

 

Moe123 wrote:
I am just asking you on behalf of many people here to respect the others

 

Don't presume to speak on behalf of other people whose opinion you don't know, ok?

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

El Tangas wrote:

I shouldn't be feeding this, but...

 

 

Moe123 wrote:

I am just asking you on behalf of many people here to respect the others

 

 

Don't presume to speak on behalf of other people whose opinion you don't know, ok?

 

English: Many doesnt mean all!.

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

Ok this stops here.

Topic locked