cli/sei – virtual problem?

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

In avr-libc manual I've fund an interesting article by Jan Waclawek abort reordering problems in gcc.
Let's look at the following code from manual:

unsigned int ivar;

void test2( unsigned int val )
{
  val = 65535U / val;

  cli();

  ivar = val;

  sei();
}

The generated assembly is as follows:

00000112 :
 112:	bc 01       	movw	r22, r24
 114:	f8 94       	cli
 116:	8f ef       	ldi	r24, 0xFF	; 255
 118:	9f ef       	ldi	r25, 0xFF	; 255
 11a:	0e 94 96 00 	call	0x12c	; 0x12c <__udivmodhi4>
 11e:	70 93 01 02 	sts	0x0201, r23
 122:	60 93 00 02 	sts	0x0200, r22
 126:	78 94       	sei
 128:	08 95       	ret

We can clearly see that cli was reordered. But to me the whole concept of this example is unclear. If we need an atomic access to ivar, it means that ivar can by modified or accessed in unpredictable way in other places in program, probably in interrupt routines. So ivar declaration is bad, it should be as follows:

volatile unsigned int ivar;

But in that case the generated assembly code is perfect:

volatile unsigned int ivar;

void test2( unsigned int val )
{
 172:	bc 01       	movw	r22, r24
  val = 65535U / val;
 174:	8f ef       	ldi	r24, 0xFF	; 255
 176:	9f ef       	ldi	r25, 0xFF	; 255
 178:	0a d0       	rcall	.+20     	; 0x18e <__udivmodhi4>

  cli();
 17a:	f8 94       	cli

  ivar = val;
 17c:	70 93 03 01 	sts	0x0103, r23
 180:	60 93 02 01 	sts	0x0102, r22

  sei();
 184:	78 94       	sei
}
 186:	08 95       	ret

So am I wrong and missing the point, or above example is a crap?

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

You are correct that ivar must be declared volatile. In most cases doing so will result in correct code. However, I believe that there may still be some circumstances that may reorder the code.

Regards,
Steve A.

The Board helps those that help themselves.

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

The fundamental problem is that the "volatile" in "asm volatile"
doesn't mean the same thing it does in "volatile unsigned int".
Thus, though it rarely happens,
undesired reordering of cli(), sei(), =SREG and SREG=
with respect to other statements is allowed.
rarely != never.

Moderation in all things. -- ancient proverb

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

Yes, I understand it, but the example form AVR-libc manual works fine. So it should be removed or corrected. And BTW, I’m looking for a working example of such reordering.

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

This thread is the "origin" of the discussion, I believe:
https://www.avrfreaks.net/index.p...

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 have read the topic, but it is obviously based on a bad example – variable which should be declared as volatile isn’t. So all results are bad, and interpretation is not possible. I know that reordering of sei/cli can be a problem, but I cannot find any working example of it. And AVR-libc documentation (Jan’s article) is wrong, and must be corrected, or even deleted if we cannot find any example of such reordering.

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

The example s***s volatile-wise, true. A carefully supplied snippet of volatile "fixes" it - see https://www.avrfreaks.net/index.p... (from an "even more original" thread to the problem).

But, if you think about it, the fact that volatile "fixes" the example is even MORE disturbing than the original problem, because we don't have an explanation WHY did it "fix" it! Why adding "volatile" somewhere around the ivar variable would prevent reordering of cli() and the multiplication, i.e. unrelated (or only indirectly related) statements? What is the exact mechanism of volatile preventing these reorderings? How "far" does the "magic" of volatile extend in more complex cases with more statements within the cli()/sei()? How does the exact form of cli()/sei() (i.e. whether it has volatile and "memory" in its definition or not) influence this "magic"?

There appear to be examples of unexpected reordering even with volatile around - see the OP in abovementioned thread - but they appear to be nontrivial, lengthy and impossible to reduce to a few lines.

So, the above example serves as "smell of smoke". For me, that means fire nearby. I know that the younger generation likes to work along the "I saw it working so it must be correct" lines but I despise that approach; call me a grunting elder if you want.

Jan Waclawek

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

Jan, the problem is that everybody here talks a lot about reordering, but in properly composed program I cannot see the problem. It doesn’t mean that the problem does not exist. I will be happy if somebody can provide (event if not trivial) program which can show the problem. Without it your article in AVR-libc documentation is pointless.

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

TFrancuz wrote:
Jan, the problem is that everybody here talks a lot about reordering, but in properly composed program I cannot see the problem.
Well, define "properly composed program" and show the mechanism, how does it prevent the reordering. I presume that would involve study of relevant portions of the gcc sources - I admit I am not versed in C/C++ and programming in general to be able to do that. I will gladly rewrite that article then, as the main point of the article is uncertainty: presently we don't know how to prevent reordering in a consistent way.

Meantime, you can rewrite it yourself, if you wish - avr-libc is open source, you can submit patches anytime.

Jan

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

Quote:

But, if you think about it, the fact that volatile "fixes" the example is even MORE disturbing than the original problem, because we don't have an explanation WHY did it "fix" it!

I could have sworn someone had given the answer to this the other day. So I went looking for the thread and found it:

https://www.avrfreaks.net/index.p...

Rather ironically it's another of these "inspecting your navel" threads started by OP and even more ironically the person who posted the solution from the C standard was YOU. You told us it was 5.1.2.3 That explains that volatile creates sequence points.

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

No, that's NOT the answer to WHY.

The multiplication is not related to volatile access, so it may be moved across sequence points freely. Indeed, in the original example, where ival is not volatile, the multiplication is moved across the semicolon (and across cli(), but it is probably not a sequence point in itself). "Restoring the order" would make sense if we would make val volatile; but we did not.

Note that making ivar volatile does not have any direct relationship to cli() nor the multiplication. This is why the mechanism of "correcting" the ordering by making ivar volatile is unclear.

You cannot find the answer in the standard (plus adherence of gcc to the standard), as all features of cli() are non-standard, (avr-)gcc specific.

JW

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

And, just for the sake of entertainment, the following:

volatile unsigned int ivar;

void test2( unsigned int val )
{
  val = 65535U / val;

  cli();

  ivar = 1 + val;

  sei();
} 

compiles to

00000112 :
 112:	bc 01       	movw	r22, r24
 114:	f8 94       	cli
 116:	8f ef       	ldi	r24, 0xFF	; 255
 118:	9f ef       	ldi	r25, 0xFF	; 255
 11a:	0e 94 9d 00 	call	0x13a	; 0x13a <__udivmodhi4>
 11e:	6f 5f       	subi	r22, 0xFF	; 255
 120:	7f 4f       	sbci	r23, 0xFF	; 255
 122:	70 93 01 02 	sts	0x0201, r23
 126:	60 93 00 02 	sts	0x0200, r22
 12a:	78 94       	sei
 12c:	08 95       	ret

As I said above, I see the reordering problem as a principial one, and am not going to discuss what would constitute a "properly composed program", with no proper grounds in studying/writing the gcc sources.

JW

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

My recollection is that from a theoretical perspective,
the problem has no solution.
That said, from the compiler's perspective,
cli() and sei() are just baggage it is required to
carry around even though they don't do anything.
The compiler, having no incentive to move them, usually doesn't.
Non-volatile references and assignments can be and often are moved around them.

Moderation in all things. -- ancient proverb

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

wek wrote:
.... I know that the younger generation likes to work along the "I saw it working so it must be correct" lines but I despise that approach; call me a grunting elder if you want.
I don't think you are being a grunting elder, you are just being sensible.

This issue, which has been called the "normalization of deviance" (almost always there are clues that things aren't quite right), afflicts all ages and led to the loss of not one, but two space shuttles -- even though it had been identified as a root cause after the first one exploded.

- John

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

wek wrote:
The example s***s volatile-wise, true. A carefully supplied snippet of volatile "fixes" it - see https://www.avrfreaks.net/index.p... (from an "even more original" thread to the problem).

But, if you think about it, the fact that volatile "fixes" the example is even MORE disturbing than the original problem, because we don't have an explanation WHY did it "fix" it! Why adding "volatile" somewhere around the ivar variable would prevent reordering of cli() and the multiplication, i.e. unrelated (or only indirectly related) statements? What is the exact mechanism of volatile preventing these reorderings? How "far" does the "magic" of volatile extend in more complex cases with more statements within the cli()/sei()? How does the exact form of cli()/sei() (i.e. whether it has volatile and "memory" in its definition or not) influence this "magic"?

As I noted elsewhere,
volatile variables do not keep cli() and sei() from being moved.

They are rarely ever moved because of inertia.
The compiler does not find them interesting enough to move.

volatile keeps references to variables from being moved.
That is why combinations of volatile, cli()
and sei() usually work as we would like.

Counter-examples will probably be compiler-version-dependent.

Quote:
There appear to be examples of unexpected reordering even with volatile around - see the OP in abovementioned thread - but they appear to be nontrivial, lengthy and impossible to reduce to a few lines.

So, the above example serves as "smell of smoke". For me, that means fire nearby. I know that the younger generation likes to work along the "I saw it working so it must be correct" lines but I despise that approach; call me a grunting elder if you want.

Jan Waclawek

Moderation in all things. -- ancient proverb