(1<<bitnumber) expression considered harmful

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

Apologies to Edgar Dijkstra: Go To Statement Considered Harmful

 

The below has been at least mentioned a few times on 'Freaks over the years...

 

(1<<bitnumber) is now the accepted method to write e.g.

TIFR0|=(1<<TOV0);

as in the current thread http://www.avrfreaks.net/forum/t...

 

As AVR8 compilers matured, it turns into an SBI:

TIFR0|=(1<<TOV0);
000000AB  SBI 0x15,0		Set bit in I/O register

 

...and thus CodeVision's "dot" extension e.g. TIFR0.TOV0 is forever condemned to the fires of damnation.  But I digress...

 

Now, experienced 'Freaks know that the OP in the other thread really "meant" to write

TIFR0 = (1<<TOV0);

 

Running the above fragment in Studio 6.2 simulator, only TOV0 flag got cleared and not OCF0A or OCF0B.  Expected with the SBI.

 

BUT -- the code writer said to the compiler:  "Get the contents of volatile [I/O register] TIFR0, OR with 1, and write the result back".  That is not the generated code, is it?  A very dark and hidden corner in AVR8 world.  Should it be changed?  Probably not.  But is it "conforming to the C standard?"  You make the call.

 

===========

Even better (or worse, depending on your point of view):  Consider the explicit RMW; the generated code is still a single SBI:

 

		TIFR0 = TIFR0 | (1<<TOV0);
 114:	a8 9a       	sbi	0x15, 0	; 21

Right or wrong?

 

=================================

[edit]  In poking at the above I couldn't really think of a real-world example.  "Volatile" to my past lives was something like a memory-mapped peripheral where read/write strobes could cause something to happen.

 

But back to my timer0 example above.  If the above for a single-bit is OK, then what if one wants to clear two out of the three bits?  [from the screen shots in the other thread you can see that all three bits are set.]

 

 104:	93 e0       	ldi	r25, 0x03	; 3
 ...
 
		TIFR0 = (1<<TOV0) | (1<<OCF0A);
 118:	95 bb       	out	0x15, r25	; 21
		TIFR0 |= (1<<TOV0) | (1<<OCF0A);
 11a:	85 b3       	in	r24, 0x15	; 21
 11c:	83 60       	ori	r24, 0x03	; 3
 11e:	85 bb       	out	0x15, r24	; 21
		TIFR0 = TIFR0 | (1<<TOV0) | (1<<OCF0A);
 120:	85 b3       	in	r24, 0x15	; 21
 122:	83 60       	ori	r24, 0x03	; 3
 124:	85 bb       	out	0x15, r24	; 21

Now my head hurts.  More than one bit and |= does a RMW, but not for a single bit.  I have no idea what is right or wrong.

 

 

 

 

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: Thu. Aug 11, 2016 - 08:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A lot depends on how one uses those flags. If I want to clear all of the flags I might do TIFR0 = 0xFF;. Typically, I rely on the interrupt routine to clear the flags. I a few cases I may want the ability to clear just a single flag so I have no problem with TIFR0 |= (1<<TOV0); yielding a SBI and leaving the other bits alone. I guess I'm just used to it...

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

Not being well versed in the subtleties of C, I opt for execution speed and code compactness over the strict adherence to language conventions, especially under the hood. Like frog_jr, I mostly rely on ISR execution to deal with interrupt flag clearing. And, when that is not the case, as in polling, writing one bit almost always does what needs to be done. Guess I'm not going to get my undies all twisted!

 

Jim

Jim Wagner Oregon Research Electronics, Consulting Div. Tangent, OR, USA http://www.orelectronics.net

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

I a few cases I may want the ability to clear just a single flag so I have no problem with TIFR0 |= (1<<TOV0); yielding a SBI and leaving the other bits alone. I guess I'm just used to it...

The trouble there is that you're >>relying<< on the toolchain to turn that into a single bit twiddling instruction.  If you port the code to another compiler, or even another device where TIFR0 is not reachable by sbi/cbi, or even the same device but using a different timer where TIFRn is not reachable, then the results will not be what you expect, especially if your code relies on other bits in that register.

 

"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."

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

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

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

 

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

In the case where all three flags in TIFR0 are set, the

TIFR0 |= (1<<TOV0) | (1<<OCF0A);

will actually clear all three bits since it is doing the RMW

 11a:	85 b3       	in	r24, 0x15	; 21   // r24 would be 0x07
 11c:	83 60       	ori	r24, 0x03	; 3    // no change
 11e:	85 bb       	out	0x15, r24	; 21   // clear all three flags...

while the

TIFR0 = (1<<TOV0) | (1<<OCF0A);

does what I expect (just clear OVF and OCFA, leaving OCFB alone).

So, beware using |= for clearing multiple flag bits.

Last Edited: Thu. Aug 11, 2016 - 08:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@joeymorin, I do not disagree, I [always] use = when dealing with flag registers.

I was just commenting that I do not mind that |= behaves the way it does (for low I/O) creating an SBI.

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

the code writer said to the compiler:  "Get the contents of volatile [I/O register] TIFR0, OR with 1, and write the result back".  That is not the generated code, is it? 

 Yet it might be the generated hardware action.  It does generate a two-cycle RMW of the IO register...

This has bothered me a bit, especially since in some architectures some registers have bits that are CLEARED by writing a 1: "INTSTATUS = INTSTATUS; // clear all pending interrupts"

OTOH, not so much AVR, and it IS an optimization that I want to have happen without needing special "sbi()" extensions.

 

Note that this has more to do with the operation of "|=" rather than "1<<bitno"...

 

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

westfw wrote:
It does generate a two-cycle RMW of the IO register...

Now you get into AVR history.  "Classic" AT90 did indeed have the hidden RMW.  But check a modern datasheet...

 

Some of the Status Flags are cleared by writing a logical one to them. Note that, unlike most
other AVRs, the CBI and SBI instructions will only operate on the specified bit, and can therefore
be used on registers containing such Status Flags. The CBI and SBI instructions work with registers 0x00 to 0x1F only.
 

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

Now my head hurts. .....I have no idea what is right or wrong.

I have an appointment for my pension application today...maybe you should try the same thing..... devil

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

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

Or you could just write it out in assembly to start with...  wink

 

S.

 

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

On PORTx, DDRx, GPIOx and some other registers,

one can argue sensibly about whether generating an SBI instruction for an |= violates the volatile rules.

I've done so.

To summarize my position: It does, but the compiler should do it anyway.

On PINx and interrupt flag bits, there is only one sensible position:

The compiler is wrong.

I'm not convinced that *any* assignment to any of those registers is valid C.

In no case, does the hardware do the assignment.

That said, all whole-register read mechanisms produce the same result

and all whole-register write mechanisms produce the same result,

so whole-register optimization choices have no effect, except for timing, on the result.

|=ing a single PINx or interrupt flag bit can produce different results

depending on whether whole-register or single-bit instructions are used.

As noted before, in neither case does the hardware do the assignment.

I think the best answer for this is a pair of macros that expand

into in-line assembly containing a single SBI or CBI instruction.

 

PORTB ^= fred;

Is the compiler allowed to compile that as

OUT PORTB, Rfred

?

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

Last Edited: Fri. Aug 12, 2016 - 07:30 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would agree that |= or &= are both RMW statements.    That is what you are asking the Compiler to produce.

 

As such,   Compilers should not generate SBI or CBI.

 

Hey-ho,   most people,  most of the time use them on SFRs where SBI, CBI produce what they wanted.

 

A few people totally ignore the clear advice in the data sheet to not RMW on interrupt flags.

It just happens that the Compiler's erroneous SBI happens to avoid the consequences of RMW on interrupt flags.

 

@skeeve,

^= is also RMW.    It should certainly use IN, XOR, OUT or LDS, XOR, STS sequences for a PORTx

If the Compiler was incredibly clever,   I suppose that it could generate OUT or STS for a PINx if it had detected a compatible targets.

 

David.

Last Edited: Fri. Aug 12, 2016 - 08:04 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I thought SBI was a single instruction read-modify-write. 

274,207,281-1 The largest known Mersenne Prime

In that awkward stage between preschool and death

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

It used to be. The old models (mega 8, 16, 32 etc) used to describe it as such. But that appears to have changed for later model (from about 2005..2006 onwards and mega48/88/168 on).

 

I'm just guessing that when the code generation in GCC that creates SBI/CBI was first done the author may have been relying on the fact that the documentation at that point was suggesting RMW.

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

Well,   that is easy enough to test.   I have mega16 and mega32.   I just need to set both OCF1A and TOV1 by just running Timer1 in mode #0

 

Then using SBI,  I can see if one flag clears or both flags clear.

 

In fact I can even do this in the Simulator.

 

The mystery question is:  what does an AT90S1200 or an AT90S2313 do ?

An ATS1200 does not have multiple Interrupt Flags.  But the AT90S2313 does.

 

I would guess that someone out there has got a 90S2313 or 90S8535.   I have not.

 

David.

Last Edited: Fri. Aug 12, 2016 - 01:40 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Woo-hoo.  That was easy.

 

/*
 * test_SBI_RMW.c
 *
 * Created: 12-Aug-16 16:04:59
 * Author : David Prentice
 */ 

#include <avr/io.h>

#if !defined(TIFR1)
#define TIFRX TIFR
#else
#define TIFRX TIFR1
#endif

int main(void)
{
    volatile uint8_t flags;
	OCR1A = 1;
	TCCR1B = (1<<CS10);
	while (1)
    {
	    flags = TIFRX & ((1<<TOV1)|(1<<OCF1A));
		if (flags == ((1<<TOV1)|(1<<OCF1A))) {
			TIFRX |= (1<<TOV1);
			asm("nop");
		}
    }
}

Run the Simulator.   For a mega16 all the interrupt bits are cleared.

For a mega164P only the TOV1 flag is cleared.

 

So Cliff was 100% correct.  In fact I think I will try this with a Tiny2313 and a Tiny2313A.

 

David.

 

Edit.   Yes, t2313, t2313A, t4313, t1634 all do a RMW (at least in the Simulator).  i.e. both flags are cleared.

 

I am intrigued.  Perhaps I will try this with real hardware.   However the Simulator is supposed to be accurate.

Last Edited: Fri. Aug 12, 2016 - 03:34 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
So Cliff was 100% correct. In fact I think I will try this with a Tiny2313 and a Tiny2313A.

If I were to hazard a guess, the models with the pin-toggle write to the PIN register will also have the non-RMW SBI/CBI.

 

Now, I should have made this a poll...

 

-- Are compilers (I checked GCC and CodeVision) wrong to turn

TIFR0|=(1<<TOV0);

into an SBI?

 

-- Can you think of a real example for someone knowledgeable about AVR8 flag registers to actually want to do that?

 

-- If answer is no to previous question, does it matter?  As said in #11

 

skeeve wrote:
violates the volatile rules. ... It does, but the compiler should do it anyway.

 

 

 

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

Are compilers (I checked GCC and CodeVision) wrong to turn

TIFR0|=(1<<TOV0);

into an SBI?

Is your suggestion that they should recognise the target as being a flag register specifically and generate different code than for a target such as PORTB? I think most people would prefer the efficiency of SBI/CBI for the port bits they access a lot at the penalty of needing to think carefully about the use of |= for lesser used flag registers. 

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

clawson wrote:
Is your suggestion that they should recognise the target as being a flag register specifically and generate different code than for a target such as PORTB? I think most people would prefer the efficiency of SBI/CBI for the port bits they access a lot at the penalty of needing to think carefully about the use of |= for lesser used flag registers.

 

I'm not making a suggestion one way or the other.  As an AVR8 app writer, indeed I want SBI/CBI for my PORTB work.

 

What I'm asking is whether a compiler that is supposedly playing by the rules of C; standard-conforming; similar -- is right to ignore explicit RMW "orders" from the programmer on a "volatile" item?

 

I can think of no side effects or any different operation with SBI on PORTB work [on modern models as discussed earlier], and indeed has advantages in >>not<< introducing RMW considerations.

 

As outlined above, there is a possible side effect with TIMSK0.   But again, is it moot since we wouldn't tell the compiler to do that in the first place?

 

 

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: Fri. Aug 12, 2016 - 04:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
What I'm asking is whether a compiler that is supposedly playing by the rules of C; standard-conforming; similar -- is right to ignore explicit RMW "orders" from the programmer on a "volatile" item?

Perhaps.  A bit of N1256 standard:

An object that has volatile-qualified type may be modified in ways unknown to the
implementation or have other unknown side effects. Therefore any expression referring
to such an object shall be evaluated strictly according to the rules of the abstract machine,
as described in 5.1.2.3. Furthermore, at every sequence point the value last stored in the
object shall agree with that prescribed by the abstract machine, except as modified by the
unknown factors mentioned previously.116) What constitutes an access to an object that
has volatile-qualified type is implementation-defined.

 

 Is the highlighted sentence the "out" that lets AVR C compilers do what is best?

 

I'm not good enough at interpreting the standard to fully understand the "abstract machine".  A few paragraphs that I thought might be pertinent:

Really not a big deal to me.  I saw the statement in the linked thread when examining the posted code looking for OP's problem situation.  It got me to thinking...

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:
Now, I should have made this a poll...

 

-- Are compilers (I checked GCC and CodeVision) wrong to turn

TIFR0|=(1<<TOV0);

into an SBI?

 

-- Can you think of a real example for someone knowledgeable about AVR8 flag registers to actually want to do that?

 

-- If answer is no to previous question, does it matter?  As said in #11

 

skeeve wrote:
violates the volatile rules. ... It does, but the compiler should do it anyway.
Note that this quote does not apply to TIFR0.

The compiler could and possibly should convert TIFR0=(1<<TOV0) into an SBI.

So far as I know, none do.

None of the C statements under discussion are valid.

The represented assignments cannot be done on the machine.

TIFR0=0 is valid, but the compiler-generated code would likely be incorrect.

 

#define sbi1(reg, bit) asm( " SBI %0, %1\n" :: I(_SF_IO_ADDR(reg)), I(bit)

#define cleariflag(reg, bit)  do { if(_SF_IO_ADDR(reg)<=0x1f) sbi1(reg, bit); else reg=(1<<(bit); } while(0)

 

sbi1 instead of sbi because the latter already exists.

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

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

skeeve wrote:

theusch wrote:
Now, I should have made this a poll...

 

-- Are compilers (I checked GCC and CodeVision) wrong to turn

TIFR0|=(1<<TOV0);

into an SBI?

 

-- Can you think of a real example for someone knowledgeable about AVR8 flag registers to actually want to do that?

 

-- If answer is no to previous question, does it matter?  As said in #11

 

skeeve wrote:
violates the volatile rules. ... It does, but the compiler should do it anyway.
Note that this quote does not apply to TIFR0.

The compiler could and possibly should convert TIFR0=(1<<TOV0) into an SBI.

So far as I know, none do.

None of the previous C statements under discussion are valid.

The represented assignments cannot be done on the machine.

TIFR0=0 is valid, but the compiler-generated code would likely be incorrect.

 

#define sbi1(reg, bit) asm( " SBI %0, %1\n" :: I(_SF_IO_ADDR(reg)), I(bit)

#define cleariflag(reg, bit)  do { if(_SF_IO_ADDR(reg)<=0x1f) sbi1(reg, bit); else reg=(1<<(bit); } while(0)

 

sbi1 instead of sbi because the latter already exists.

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

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

Why should the TIFR assignment be turned into SBI? For a start it would kill every mega8/16/32 or Tiny app.
.
What is wrong with simply obeying the datasheet? i.e. an assignment uses OUT / STS.
You clear interrupt flags with = and not with |=
.
Many other cpus work in the same way. It may be confusing to beginners but it is just something you have to accept.
.
I don't understand why the SBI behaviour changed with the 328/324 chips.

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

skeeve wrote:
None of the previous C statements under discussion are valid.

You lost me.  "TIFR0 |= (1<<TOV0);" is not valid C?

 

Nor the simplified "TIFR0 |= 1;" ?

 

Nor the expanded "TIFR0 = TIFR0 | (1<<TOV0);" ?

 

The compiler could and possibly should convert TIFR0=(1<<TOV0) into an SBI.

A bit different direction from my original query,  but as pointed out above the SBI will have hidden RMW side effects on Mega16-generation AVRs and clear all set bits in the flag register.

 

==============

Again, I don't think the situation would arise much in a real app.  I had thoughts perhaps analogous to your sbi1 -- perhaps have "volatile", and "really volatile" where the code generator is trained to practice safe flag bit clearing.  Dunno.  Not worth the effort IMO.

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

avr-libc (or was it part of avr-gcc) used to include sbi() and cbi() functions (or macros; I dunno.)

What was the motivation for removing/deprecating them (in favor of "reg |= bitmask;")?

I mean, if the compiler always does the same thing with the reg|=bitmask form, you can trivially use that anyway, whereas it seems much more difficult to say "No, I really wanted read, or, and re-write" if the optimization is always applied...

 

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

As far as I can see,  the |= statement in C clearly implies Read-Modify-Write.

And the historic AVR SBI instruction implements the C functionality.

 

C does not seem to have a specific SEB or CLB statement.   Many microcontrollers have SEB, CLB functionality.

 

C has always been amenable to adding extras with a library.   e.g. intrinsic functions.

 

Quite honestly,  the only practical problems on an AVR is with TIFR or TWCR.   e.g. when an interrupt flag is cleared with a 1.

If you obey the advice in the datasheet,   everything works just fine.    Not only that.   An OUT or STS instruction is atomic.   And faster than any SBI or LDS/ORI/STS sequence.

 

So why did Atmel change the SBI functionality?

 

David.

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

westfw wrote:

avr-libc (or was it part of avr-gcc) used to include sbi() and cbi() functions (or macros; I dunno.)

What was the motivation for removing/deprecating them (in favor of "reg |= bitmask;")?

I mean, if the compiler always does the same thing with the reg|=bitmask form, you can trivially use that anyway, whereas it seems much more difficult to say "No, I really wanted read, or, and re-write" if the optimization is always applied...

 

The joy of revision control is that if you really want to know the answer you can wind back time and take a look.  I haven't but I suspect that in the early days the compiler may not have been smart enough to optimise  to SBI/CBI when it could (1 bit changing and low enough address) so I'm guessing sbi() and cbi() were Asm macros that the user could manually force the use of SBI/CBI my when they knew it was appropriate. Later,  when the compiler automated this they were simplified to "do nothing" macros. 

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

david.prentice wrote:
So why did Atmel change the SBI functionality?

???  That would seem fairly obvious to me -- to create a mechanism to modify [individual] port bit values without introducing possible RMW side effects.  Unless I'm way off.

 

It more-or-less seems to coincide with the timeline of write-to-PIN-to-toggle.  In practice that doesn't come up often, but prior it is painful to toggle an AVR8 I/O port bit.

 

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

Oops.   I just wrote and Simulated a trivial C program without inspecting the generated ASM.

 

I modified my test program to light a real LED on a real target:

int main(void)
{
    volatile uint8_t flags;
	OCR1A = 1;
	TCCR1B = (1<<CS10);
	DDRB |= LED_bm;
	while (1)
    {
	    flags = TIFRX & ((1<<TOV1)|(1<<OCF1A));
		if (flags == ((1<<TOV1)|(1<<OCF1A))) {
			TIFRX |= (1<<TOV1);
			if (TIFRX == 0) PORTB |= LED_bm;
			asm("nop");
		}
    }
}

 

It all comes down to the address of TIFRX.   If within SBI range,  it uses SBI.   If not,  it uses IN/ORI/OUT or LDS/ORI/STS. e.g. for mega328P

			TIFRX |= (1<<TOV1);
00000054  SBI 0x16,0		Set bit in I/O register
			if (TIFRX == 0) PORTB |= LED_bm;
00000055  IN R24,0x16		In from I/O location
00000056  CPSE R24,R1		Compare, skip if equal
00000057  RJMP PC+0x0002		Relative jump
--- No source file -------------------------------------------------------------
00000058  SBI 0x05,0		Set bit in I/O register
00000059  NOP 		No operation

But a Mega16 has got TIFR above 0x20.  

 

			TIFRX |= (1<<TOV1);
00000047  IN R24,0x38		In from I/O location
00000048  ORI R24,0x04		Logical OR with immediate
00000049  OUT 0x38,R24		Out to I/O location
			if (TIFRX == 0) PORTB |= LED_bm;
0000004A  IN R24,0x38		In from I/O location
0000004B  CPSE R24,R1		Compare, skip if equal
0000004C  RJMP PC+0x0002		Relative jump
--- No source file -------------------------------------------------------------
0000004D  SBI 0x18,0		Set bit in I/O register
0000004E  NOP 		No operation

And so does a Tiny2313

			TIFRX |= (1<<TOV1);
0000002B  IN R24,0x38		In from I/O location
0000002C  ORI R24,0x80		Logical OR with immediate
0000002D  OUT 0x38,R24		Out to I/O location
			if (TIFRX == 0) PORTB |= LED_bm;
0000002E  IN R24,0x38		In from I/O location
0000002F  CPSE R24,R1		Compare, skip if equal
00000030  RJMP PC+0x0002		Relative jump
--- No source file -------------------------------------------------------------
00000031  SBI 0x18,0		Set bit in I/O register
00000032  NOP 		No operation

So perhaps SBI has always been non-RMW.    Someone would have to try it on an obsolete AT90S part.

It seems more likely that SBI has never been RMW.    Compilers should never really translate |= into SBI.

 

The different behaviour of the "newer" AVRs is down to putting TIFR into SBI address space.

 

And if you follow the datasheet advice,   you can clear individual flags in TIFR with an = statement.

Faster, atomic,  and easier.

 

David

Last Edited: Sat. Aug 13, 2016 - 03:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Does any of this support Lee's subject line of "(1<<bitnumber) expression considered harmful"? Quirky maybe, but "harmful"?

 

Jim

Jim Wagner Oregon Research Electronics, Consulting Div. Tangent, OR, USA http://www.orelectronics.net

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

david.prentice wrote:

As far as I can see,  the |= statement in C clearly implies Read-Modify-Write.

 

I think I disagree. According to the C semantic model, provided the right bits are set in the lvalue then how they are set is implementation dependent.

 

C doesn't know anything about registers, which is a major deficiency for low level programming. Even volatile doesn't really have the semantics we need,  there is a lot dependent on the implementation.

 

Frankly, C is an ancient language which should be binned, but the nature of software is we are stuck with the past. At least, there needs to be formal attributes and semantics defined for things like register access, RMW behaviour, memory barriers, instruction ordering etc.

Bob.

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

Surely |= is just a shortened form of :
variable = variable | expression;
.
Which implies to me that you read variable, OR it with expression, assign the result to variable.
.
David.

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

david.prentice wrote:
Surely |= is just a shortened form of : variable = variable | expression; . Which implies to me that you read variable, OR it with expression, assign the result to variable. . David.

 

No, the semantic model describes the result not the method. In the specific case of |=, the operation can only lead to bits being set. Their previous value is a don't care, therefore there is no need to read them. volatile means that if an access is required, then the value is re-read. The only reason for the read is to preserve untouched bits, but if the compiler can preserve the bits in a more optimal way, it can do it.

 

A valid implementation for a  |= b might be

 

for each bit in b

   if bit is set

      set corresponding bit in a with "set bit" instruction

 

No read of a required! Whether that is most optimal code is up to the compiler writer.

 

Access to registers that require special operation MUST be done with assembler to ensure correct operation, otherwise you are in the alligator infested swamp of "implementation defined behavior".

 

 

The thread title should probably read "implementation defined behavior considered harmful", but I think you can easily generalize that to: "C language considered harmful" :)

 

 

Edit: volatile must preserve the read and write regardless, because they are observable side-effects.

Bob.

Last Edited: Sun. Aug 14, 2016 - 10:55 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

donotdespisethesnake wrote:
volatile means that if an access is required, then the value is re-read.

 

With further study, I am going to disagree with myself. The quote in #21 is key.

 

An expression such as  a |= b is indeed according to the standard, nearly equivalent to a = a | b. If a is volatile, then the compiler must issue a read of that variable within the applicable sequence point. An "access" of a volatile is an "observable side-effect", which must be preserved, and "access" includes to read or modify (write).

 

You can imagine that a volatile is a variable that generates a random number when you read it. Even if the value is not used, the compiler must read it. In the statement "a = a | 0;" if a is volatile, then the read and write are observable side-effects, so the compiler must do both, even if numerically the value will not be changed. The statement could be optimized to "a = a;" however.

 

So I conclude that if sbi is not a read-modify-write operation, then generating an sbi instruction for a volatile memory location is a bug. According to my reading of the C standard and the web, the compiler should ensure a read of the whole variable, and then write back the whole variable.

 

Only if a location is not declared volatile, can the compiler generate sbi (regardless of optimisation).

 

Reference : http://www.open-std.org/jtc1/sc2...

Bob.

Last Edited: Sun. Aug 14, 2016 - 11:03 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 donotdespisethesnake wrote:

So I conclude that if sbi is not a read-modify-write operation, then generating an sbi instruction for a volatile memory location is a bug.

 

theusch wrote:

 

Perhaps.  A bit of N1256 standard:

    An object that has volatile-qualified type may be modified in ways unknown to the
    implementation or have other unknown side effects. Therefore any expression referring
    to such an object shall be evaluated strictly according to the rules of the abstract machine,
    as described in 5.1.2.3. Furthermore, at every sequence point the value last stored in the
    object shall agree with that prescribed by the abstract machine, except as modified by the
    unknown factors mentioned previously.116) What constitutes an access to an object that
    has volatile-qualified type is implementation-defined.

 

 

"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."

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

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

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

 

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

joeymorin wrote:

 donotdespisethesnake wrote:

So I conclude that if sbi is not a read-modify-write operation, then generating an sbi instruction for a volatile memory location is a bug.

 

theusch wrote:

 

Perhaps.  A bit of N1256 standard:

    An object that has volatile-qualified type may be modified in ways unknown to the
    implementation or have other unknown side effects. Therefore any expression referring
    to such an object shall be evaluated strictly according to the rules of the abstract machine,
    as described in 5.1.2.3. Furthermore, at every sequence point the value last stored in the
    object shall agree with that prescribed by the abstract machine, except as modified by the
    unknown factors mentioned previously.116) What constitutes an access to an object that
    has volatile-qualified type is implementation-defined.

 

 

 

I latched onto that at first, but it's a puzzling statement because it contradicts the rest of the spec, and even the two preceding sentences. Access is clearly defined in the abstract machine:

 

3.1
1 access
〈execution-time action〉 to read or modify the value of an object
2
NOTE 1 Where only one of these two actions is meant, ‘ ‘read’ ’or‘ ‘modify’ ’is used.
3
NOTE 2 "Modify’ ’includes the case where the new value being stored is the same as the previous value.
4
NOTE 3 Expressions that are not evaluated do not access objects.
 

The abstract machine model is clear, an access is either a read or modify.

 

I think it is bad wording, I think what it means is that the instructions available in the ISA such as move, store, load, pop, push etc are examined to see which constitute read or modify operations. In this case, sbi can be either a modify, or a read-modify, depending on CPU.

 

So the statement "a |= b;" where a is volatile, according to the spec, generates required operations in the abstract machine:

 

read a

or a with b

store result in a

 

If sbi is RMW, then the 3 ops can be replaced with a single sbi, and that satisfies the read and write. However, if sbi is just modify and does not read, then the compiler should issue

 

read a

sbi a,b

 

to satisfy the abstract model.

 

So I suspect that originally the compiler was issuing a RMW sbi which was valid, but if the instruction has changed in meaning, then the compiler no longer generates instructions which match the abstract machine model.

 

Perhaps someone with a simulator can verify what code is generated for

 

volatile uint8_t a;

a = a;

Bob.

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

donotdespisethesnake wrote:
Perhaps someone with a simulator can verify what code is generated for

??? Why does one need a simulator to test generated code?

 

And shouldn't you be much more specific in ths request with respect to toolchain, version, and compile options?

 

========================

Apparently my original query isn't as cut-and-dried as first respondents seemed to indicate.  ;) 

 

I agree with those that seemed to say "we want the AVR8 code generation to stay the same for bit manipulation".

 

I can't see that anyone found a valid reason for TIMSK0 |= 1; on an AVR8.  Thus it is a "don't care"?

 

And for low port register work, there aren't the hidden side effects as with a flag register.  So again a "don't care"?

 

 

 

 

 

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

I was initially under the impression that the SBI opcode changed its functionality between AT90S and ATMEGA.
.
I have now concluded that the TIFR behaviour is simply down to its address. e.g. the mega328 has now got TIFRx in the SBI addressable area.
.
if you obey the datasheet, TIFR = (1< It just so happens that TIFR1 |= (1< .
My personal view is that a Computer Language should be as intuitive as possible.
At the same time, you should be "aware" that certain hardware registers behave differently to regular read-write memory.
.
If you follow datasheet advice, |= gives you no surprises.  = gives no surprises either.
.
David.

Last Edited: Sun. Aug 14, 2016 - 09:54 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I can't see that anyone found a valid reason for TIMSK0 |= 1; on an AVR8.  Thus it is a "don't care"?

The one way I can imagine it >>might<< be an issue is when developing with the Arduino environment.  There are a multitude of libraries in use out there, some good, some not so much.  Many use interrupts.  Many users may not know the potential issues.  I expect it wouldn't take very long to find a number of threads both here and in the Arduino fora where a user made changes to TIMSKn with a direct assignment, only to find that some other part of their code stopped working.  But I agree that this isn't of primary concern, even though the Arduino may represent a far larger user base and a far larger installed device base than nearly any other.

 

I latched onto that at first, but it's a puzzling statement because it contradicts the rest of the spec, and even the two preceding sentences. Access is clearly defined in the abstract machine:

The behaviour of the abstract machine, at least insofar as your quoted text reveals, is blind to the notion of volatile.  The section you quote makes no reference to volatile in any way.  The whole notion of volatile was bolted onto the language after the structure of the C language was framed from the perspective of an abstract machine.  It's right there in the first mention of an abstract machine:

In section 5.1.2.3, you find:

  1. The semantic descriptions in this International Standard describe the behavior of an
    abstract machine in which issues of optimization are irrelevant.

Volatile is meant to address the issues created by the abstract machine's ignorance of hardware-level details of implementation.  In my view, aspects of the language described in sections of the standard pertaining to the concept of volatile must naturally supersede and potentially contradictory statements made elsewhere, including those pertaining to the abstract machine.

 

There seems to be no other reading possible.  The text seems clear and unambiguous.  That which constitutes a volatile access (i.e. a read or a write) is implementation defined.  That means an implementation can stipulate that in order for a volatile object to be read it must be sent by the local postal carrier, and that for it to be written it must be ground into a fine paste.  I can't conceive of an implementation or hardware where this stipulation would be useful, but it seems to me the point of the wording is to account for future hardware with characteristics not foreseen by the writers of the language or of the standard. 

 

Indeed, any phrase in the standard which included the words 'implementation defined' would seem to supersede any previous seemingly contradictory directives.  Volatile itself was introduced into the language specifically as a means to deal with details of hardware implementation with which an abstract machine cannot and should not concern itself.

 

One must remember that the standard is not the language, nor vice versa.  The language came first.  The standard came later as an attempt to formalise the language.  The situation is akin to music theory.  Nobody decided what was going to constitute Jazz until long after the genre first appeared, was performed, and appreciated for the innovating thing that it was.  Yet there are endless discussions even today about what is and is not Jazz.  It's an academic argument.  That doesn't make it an worthless argument, but it does occur at some distance from the business end of the music itself.   C is the same as any human language.  Language theory attempts to formalise the unregulated process of language evolution.  A computer language may begin life in a more structured way than a spoken language, but the same language processes still apply.  If the standard were the be-all-end-all of the language, then we wouldn't need newly revised standards.  To date, C has seen several.  A language is developed, put into use, and shortcomings are encountered.  Enhancements are conceived, developed, agreed upon, and formalised via a standard.  The newly refined language is put into use, and shortcomings are encountered.  Rinse and repeat.  Volatile is but one example of an agreed-upon formalised enhancement meant to address a previous shortcoming.  Its nature was debated and compromises were reached.

 

Whether or not my assessment of the wording of the standard is correct, and whether or not you or anyone agrees with it, it seems clear that the notion of volatile is inadequate to the task to which it is being put, i.e. the bit-oriented nature of sbi/cbi.  The notion of a single bit does not exist in C, so to me this is unsurprising.  Ultimately, if you need careful control of hardware-specific access, there is no provision of the C language which can meet that requirement completely.  If there were, it would seem that there wouldn't be such a great variety of compilers which permit some level of integration with assembler.

 

"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."

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

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

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

 

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

Rather a lot of the IO registers, including interupt flag registers and recent PINx registers, cannot be assigned to in the manner of SRAM.

How to compiler "TIFR0=1;":

  ; valid only if bit 0 is already 1
  LDI R16, 6
  STS TIFR0, R16

How to compile "PINB=1;":

  ; works if DDRB is 0xFF
  LDI R16, 1
  STS PORTB, R16

Some assignments cannot be done at all: "PORTC=0x80;" is never valid for an atmega168.

In these cases, what a compiler should do cannot be determined from close examination of C standards.

Perhaps compilers should not compile such statements without comment.

The compiler could have a set of addresses for which ordinary assignments require comment

and a set of macros that expand to inline assembly.

 

OTOH PORTx and DDRx registers often do behave as regular SRAM.

In such cases, the standards are relevant:

N1570, C11 wrote:
In the abstract machine, all expressions are evaluated as specified by the semantics. An actual implementation need not evaluate part of an expression if it can deduce that its value is not used and that no needed side effects are produced (including any caused by calling a function or accessing a volatile object).
The as-if rule applies to volatile objects.

PORTx and DDRx and some other IO registers may be processed with SBI and CBI instructions.

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

Last Edited: Mon. Aug 15, 2016 - 05:08 AM