> On 01/06/15 05:28, Les Cargill wrote:
>> David Brown wrote:
>>> On 31/05/15 18:28, Les Cargill wrote:
>>>> David Brown wrote:
>> <snip>
>>>>
>>>
>>> If your C macros are similarly incorrect, then you are merely lucky that
>>> they have worked. Feel free to post those macros, and I will try to
>>> comment on them too.
>>>
>>
>> David - sorry.
>>
>> I had apparently missed your post in the maelstrom. I was
>> unfamiliar with the "memory clobber" idiom.
>>
>> Thanks for that, David.
>
> Sorry if I was a bit crass with you in my posts last night - it hadn't
> occurred to me that you had simply failed to see my posts in the mass in
> this thread.
>
No apology needed - I was ... quite confused :) You were quite
professional about it.
I saw your *postS*, but not the critical *post*.
As soon as I realized that there was a construct specifically for
warning the toolchain that "here we made memory changes", I realized
that that was probably the only way to reliably enforce this - this
issue had been deliberated on, and the solution provided wasn't
just there as decoration.
>>
>> For what it is worth, the reference to extern class routines and
>> variables was sufficient last time I'd faced this. As you say,
>> I'd probably been lucky.
>
> Compilers usually generate code in the order of operations in the source
> code, unless they have particular reason to re-order them. This means
> that you can very often get away with code that doesn't enforce the
> order specifically. And if you don't have LTO or other sorts of
> whole-program optimisation enabled, then external function calls form a
> barrier to a fair amount of re-ordering. Thus the external function
> calls here /usually/ work.
>
Yes. Emphasis "usually".
> However, compilers are getting smarter and smarter - and they are
> optimising over a wider range with more cross-function and cross-module
> optimisation. Techniques that worked well enough before, are now
> failing. Some people think this is a bad thing - that compilers are
> getting "smart-ass" rather than "smart".
:)
> Personally, I am a fan of
> compiler optimisation, but it means that we need to be more accurate in
> telling the compiler /exactly/ what we want. But it has the advantage
> of letting us write neater and clearer source code, knowing that the
> compiler will produce efficient object code in the end.
>
Indeed.
> As well as varying between compilers (or versions of compilers, or
> selection of optimisation flags), these things also vary between
> targets. When you are using an ARM Cortex-M3/M4 (as in this thread),
> the cpu is pretty straightforward and does one thing at a time. Thus
> the compiler can order generated instructions in the same way as the
> source code, without any penalty - this makes debugging and
> single-stepping easier, as well as being easier to understand the
> generated code. But if you are using a PowerPC, or a MIPS processor,
> especially the higher-end devices with multiple issue super-scaler cpus,
> the compiler does a great deal of re-ordering and re-scheduling to get
> maximum overlap from instructions. With such targets, you need to be
> /very/ careful about your order enforcements (and may also need
> additional assembly instructions to flush the cpu's write buffers, or
> synchronise pipelines in some way).
>
>>
>>> Do you believe that my class, as posted earlier, has errors? If so, I
>>> would like to know - while I am confident that it is correct, I would be
>>> glad to hear of any suspected problems or situations where it would not
>>> work.
>>>
>>
>> Nope - it was very clear once I'd found it. I would not have posted
>> at all had I seen that.
>>
>
> It's nice to have the confirmation.
>
Always good to find the "bug". :)
>>> But assuming it /is/ correct, why would you prefer to use a solution
>>> that is more limited in its use cases, dependent on the compiler's
>>> optimisation flags, and big and slow in the cases when it might work?
>>>
>>
>>
>> Because it had worked for me ( when we were using inline asm to
>> do something horrible ) once before. Like you say, it was
>> probably just luck that it worked.
>>
>> I did not find it particularly big and slow - the "iniline' did what
>> you'd expect. We weren't exactly tight for cycles.
>>
>> The "static" thing, though, was me not understanding the intention of
>> Tim's original code. I really thought it was just "on or off" and
>> didn't see any nesting needed.
>>
>
> Tim didn't mention nesting anywhere (at least, not that I noticed) - I
> took it as an implicit requirement (in particular, it is one of the
> reasons you need to restore the previous interrupt state rather than
> assuming interrupts are always on at the constructor).
Sure.
> Once you have
> thought of the idea of being able to nest the critical sections, it is
> obviously a useful feature. If nothing else, it helps make the class
> simpler to use.
>
Absolutely.
--
Les Cargill
Reply by David Brown●June 1, 20152015-06-01
On 01/06/15 05:28, Les Cargill wrote:
> David Brown wrote:
>> On 31/05/15 18:28, Les Cargill wrote:
>>> David Brown wrote:
> <snip>
>>>
>>
>> If your C macros are similarly incorrect, then you are merely lucky that
>> they have worked. Feel free to post those macros, and I will try to
>> comment on them too.
>>
>
> David - sorry.
>
> I had apparently missed your post in the maelstrom. I was
> unfamiliar with the "memory clobber" idiom.
>
> Thanks for that, David.
Sorry if I was a bit crass with you in my posts last night - it hadn't
occurred to me that you had simply failed to see my posts in the mass in
this thread.
>
> For what it is worth, the reference to extern class routines and
> variables was sufficient last time I'd faced this. As you say,
> I'd probably been lucky.
Compilers usually generate code in the order of operations in the source
code, unless they have particular reason to re-order them. This means
that you can very often get away with code that doesn't enforce the
order specifically. And if you don't have LTO or other sorts of
whole-program optimisation enabled, then external function calls form a
barrier to a fair amount of re-ordering. Thus the external function
calls here /usually/ work.
However, compilers are getting smarter and smarter - and they are
optimising over a wider range with more cross-function and cross-module
optimisation. Techniques that worked well enough before, are now
failing. Some people think this is a bad thing - that compilers are
getting "smart-ass" rather than "smart". Personally, I am a fan of
compiler optimisation, but it means that we need to be more accurate in
telling the compiler /exactly/ what we want. But it has the advantage
of letting us write neater and clearer source code, knowing that the
compiler will produce efficient object code in the end.
As well as varying between compilers (or versions of compilers, or
selection of optimisation flags), these things also vary between
targets. When you are using an ARM Cortex-M3/M4 (as in this thread),
the cpu is pretty straightforward and does one thing at a time. Thus
the compiler can order generated instructions in the same way as the
source code, without any penalty - this makes debugging and
single-stepping easier, as well as being easier to understand the
generated code. But if you are using a PowerPC, or a MIPS processor,
especially the higher-end devices with multiple issue super-scaler cpus,
the compiler does a great deal of re-ordering and re-scheduling to get
maximum overlap from instructions. With such targets, you need to be
/very/ careful about your order enforcements (and may also need
additional assembly instructions to flush the cpu's write buffers, or
synchronise pipelines in some way).
>
>> Do you believe that my class, as posted earlier, has errors? If so, I
>> would like to know - while I am confident that it is correct, I would be
>> glad to hear of any suspected problems or situations where it would not
>> work.
>>
>
> Nope - it was very clear once I'd found it. I would not have posted
> at all had I seen that.
>
It's nice to have the confirmation.
>> But assuming it /is/ correct, why would you prefer to use a solution
>> that is more limited in its use cases, dependent on the compiler's
>> optimisation flags, and big and slow in the cases when it might work?
>>
>
>
> Because it had worked for me ( when we were using inline asm to
> do something horrible ) once before. Like you say, it was
> probably just luck that it worked.
>
> I did not find it particularly big and slow - the "iniline' did what
> you'd expect. We weren't exactly tight for cycles.
>
> The "static" thing, though, was me not understanding the intention of
> Tim's original code. I really thought it was just "on or off" and
> didn't see any nesting needed.
>
Tim didn't mention nesting anywhere (at least, not that I noticed) - I
took it as an implicit requirement (in particular, it is one of the
reasons you need to restore the previous interrupt state rather than
assuming interrupts are always on at the constructor). Once you have
thought of the idea of being able to nest the critical sections, it is
obviously a useful feature. If nothing else, it helps make the class
simpler to use.
Reply by Les Cargill●June 1, 20152015-06-01
David Brown wrote:
> On 31/05/15 18:28, Les Cargill wrote:
>> David Brown wrote:
<snip>
>>
>
> If your C macros are similarly incorrect, then you are merely lucky that
> they have worked. Feel free to post those macros, and I will try to
> comment on them too.
>
David - sorry.
I had apparently missed your post in the maelstrom. I was
unfamiliar with the "memory clobber" idiom.
Thanks for that, David.
For what it is worth, the reference to extern class routines and
variables was sufficient last time I'd faced this. As you say,
I'd probably been lucky.
> Do you believe that my class, as posted earlier, has errors? If so, I
> would like to know - while I am confident that it is correct, I would be
> glad to hear of any suspected problems or situations where it would not
> work.
>
Nope - it was very clear once I'd found it. I would not have posted
at all had I seen that.
> But assuming it /is/ correct, why would you prefer to use a solution
> that is more limited in its use cases, dependent on the compiler's
> optimisation flags, and big and slow in the cases when it might work?
>
Because it had worked for me ( when we were using inline asm to
do something horrible ) once before. Like you say, it was
probably just luck that it worked.
I did not find it particularly big and slow - the "iniline' did what
you'd expect. We weren't exactly tight for cycles.
The "static" thing, though, was me not understanding the intention of
Tim's original code. I really thought it was just "on or off" and
didn't see any nesting needed.
>
>
--
Les Cargill
Reply by Dave Nadler●May 31, 20152015-05-31
On Sunday, May 31, 2015 at 4:35:16 PM UTC-4, David Brown wrote:
> Dave was incorrect in his complaint - as long as the class is used
> without any nesting or recursion, it will preserve the interrupt status
> on construction, and restore the previous state on destruction.
>
> However, it is misleading (and therefore bad coding) to call the
> destructor function "interruptsOn", because that is not what the
> function does - it should be called something like "interruptsRestore".
> Bad identifier naming is a crime in programming - it lead directly to
> Dave's misunderstanding here.
Yep, I thought he was re-enabling interrupts in all cases...
I've been using the ctor/dtor pattern for interrupt bracketing
and similar resource management for decades, but I'll be fooled
by poor naming every time!
Thanks for the clarification,
Best Regards, Dave
Reply by David Brown●May 31, 20152015-05-31
On 31/05/15 18:29, Les Cargill wrote:
> Dave Nadler wrote:
>> On Thursday, May 28, 2015 at 10:17:18 PM UTC-4, Les Cargill wrote:
>>> ...
>>> CProtect(void)
>>> {
>>> interruptsOff();
>>> }
>>
>> Careful Les, that's a bug if entered in a context where
>> interrupts are already disabled and must remain so...
>>
>
> "Doctor, doctor is hurts when I do that..."
>
You are not in a position to be sarcastic here!
> That is inherent in the design of this widget.
>
No, you are incorrect here. You either misunderstood Dave's comment, or
you misunderstood your own code.
Dave was incorrect in his complaint - as long as the class is used
without any nesting or recursion, it will preserve the interrupt status
on construction, and restore the previous state on destruction.
However, it is misleading (and therefore bad coding) to call the
destructor function "interruptsOn", because that is not what the
function does - it should be called something like "interruptsRestore".
Bad identifier naming is a crime in programming - it lead directly to
Dave's misunderstanding here.
Reply by David Brown●May 31, 20152015-05-31
On 31/05/15 18:28, Les Cargill wrote:
> David Brown wrote:
>> On 29/05/15 04:23, Les Cargill wrote:
>>> Tim Wescott wrote:
>>>> This is related to my question about interrupts in an STM32F303
>>>> processor. It turns out that the problem is in the compiler (or I'm
>>>> going insane, which is never outside the realm of possibility when I'm
>>>> working on embedded software).
>>>>
>>>> I'm coding in C++, and I'm using a clever dodge for protecting
>>>> chunks of
>>>> code from getting interrupted. Basically, I have a class that
>>>> protects a
>>>> block of code from being interrupted. The constructor saves the
>>>> interrupt state then disables interrupts, and the destructor restores
>>>> interrupts.
>>>>
>>>> This has been reliable for me for years, but now the destructor is not
>>>> being called. I suspect that the optimizer can't make sense of it
>>>> because of the asm statements, and is throwing it away.
>>>>
>>>> If someone knows the proper gnu-magic to tell the optimizer not to do
>>>> that, I'd appreciate it. I'm going to look in my documentation, but I
>>>> want to make sure I use the right method, and don't just stumble onto
>>>> something that works for now but should be depreciated, or is
>>>> fragile, or
>>>> whatever.
>>>>
>>>> Here's the "protect a block" class:
>>>>
>>>> typedef class CProtect
>>>> {
>>>> public:
>>>>
>>>> CProtect(void)
>>>> {
>>>> int primask_copy;
>>>> asm("mrs %[primask_copy], primask\n\t" // save interrupt
>>>> status
>>>> "cpsid i\n\t" // disable interrupts
>>>> : [primask_copy] "=r" (primask_copy));
>>>> _primask = primask_copy;
>>>> }
>>>>
>>>> ~CProtect()
>>>> {
>>>> int primask_copy = _primask;
>>>> // Restore interrupts to their previous value
>>>> asm("msr primask, %[primask_copy]" : : [primask_copy]
>>>> "r" (primask_copy));
>>>> }
>>>>
>>>> private:
>>>> volatile int _primask;
>>>> } CProtect;
>>>>
>>>> and here's how it's used:
>>>>
>>>> {
>>>> CProtect protect;
>>>>
>>>> // critical code goes here
>>>> }
>>>>
>>>
>>>
>>> There are no side effects in the destructor.
>>>
>>> Notes:
>>> - extern is not extern "C". You can dern tootin' make 'em
>>> extern "C" but I don't see the point.
>>> - SFAIK, the "extern...inline" combination works. I have
>>> no way to test it. "extern...inline' isn't critical; it's
>>> just more or less an apology for making things slightly
>>> more complex.
>>> - If you are not using separate compilation it gets
>>> even simpler.
>>>
>>> I would:
>>>
>>> #ifndef _CPROTECT_H_
>>> #define _CPROTECT_H_
>>> // CProtect.h
>>>
>>> extern void inline interruptsOn(void);
>>> extern void inline interruptsOff(void);
>>>
>>> typedef class CProtect
>>> {
>>> private:
>>>
>>> public:
>>>
>>> CProtect(void)
>>> {
>>> interruptsOff();
>>> }
>>>
>>> ~CProtect()
>>> {
>>> interruptsOn();
>>> }
>>>
>>> } CProtect;
>>> #endif
>>>
>>>
>>>
>>> // CProtect.cpp
>>>
>>> #include <CProtect.h>
>>>
>>> static int _primask;
>>>
>>> void inline interruptsOff(void)
>>> {
>>> int primask_copy;
>>> asm("mrs %[primask_copy], primask\n\t" // save interrupt status
>>> "cpsid i\n\t" // disable interrupts
>>> : [primask_copy] "=r" (primask_copy));
>>> _primask = primask_copy;
>>> }
>>>
>>> void inline interruptsOn()
>>> {
>>> int primask_copy = _primask;
>>> // Restore interrupts to their previous value
>>> asm("msr primask, %[primask_copy]" : : [primask_copy]
>>> "r" (primask_copy));
>>> }
>>>
>>
>> This looks like you believe making an external function call enforces a
>> memory order. This is simply incorrect - if the compiler knows the
>> effects the function may have, it can use that information to re-order
>> the code. In particular, it knows something about the data the function
>> could access (it cannot legally access function-local or file static
>> data if the data does not "escape" by passing pointers to it out to
>> unknown code), and it may have full access to the source code through
>> inlining or link-time optimisation.
>>
>
> It's less that than that _primask is now a side effect. Again,
> I can't test this in his environment so the idea is to suggest
> a possible direction less than a destination.
>
> It may well be that interruptsOn() doesn't quite work
> unless you write back to _primask. Or something.
>
> Others have more directly addressed direct directives to tell the
> optimizer to hold off. I also understand not wanting to do that.
Please read the posts I have made in this thread, that give complete and
correct answers to the problem, along with an explanation for why my
recommendation is correct, and as close to optimal as possible given the
ease-of-use constraint. You may also like to read other posts in this
thread.
If you would prefer a list of issues with your class here:
1. It relies on having the interruptOn and interruptOff functions in an
external module - as noted above, this is not safe.
2. It relies on a static object to hold the previous interrupt state.
This means the method cannot be nested, making it useless for the task
in hand.
3. It is seriously inefficient, meaning interrupts are disabled for much
longer than necessary, and making critical code a costly operation
rather than merely the 3 cycles or so that are actually needed. In
embedded programming, this is important.
4. Even if the functions are treated as external black boxes, calling
them will not enforce ordering on memory references which could not
legally be accessed by an external function, thus defeating the purpose
of the class.
>
>> So while the use of external functions might appear to work, and /will/
>> work in some contexts, it will work by accident of compiler flags and
>> limited compiler capabilities, rather than by design. The use of
>> "inline" (or "extern inline") here is utterly pointless
>
> Of course.
Then why is it there in the first place?
>
>> - if the
>> compiler is able to inline the code, then you lose all hope of it
>> working.
>
> Not so much. It'll work either way.
No, it will not work. Rather than repeating myself here, I direct you
to my other posts in this thread (as well as earlier in this thread)
where I have explained this. If you find flaws in something I wrote
there, please say. If you don't understand the issues, please ask.
>
>> And when the code is /not/ inlined, you generate significantly
>> bigger and slower code for something that should boil down to 2
>> instructions for the constructor and 1 instruction for the destructor.
>>
>
> Life is like that. I would have used 'C' macros myself
> because those always work, but the use of a class to do this
> is pretty slick.
>
If your C macros are similarly incorrect, then you are merely lucky that
they have worked. Feel free to post those macros, and I will try to
comment on them too.
Do you believe that my class, as posted earlier, has errors? If so, I
would like to know - while I am confident that it is correct, I would be
glad to hear of any suspected problems or situations where it would not
work.
But assuming it /is/ correct, why would you prefer to use a solution
that is more limited in its use cases, dependent on the compiler's
optimisation flags, and big and slow in the cases when it might work?
Reply by Les Cargill●May 31, 20152015-05-31
Tim Wescott wrote:
> On Thu, 28 May 2015 21:23:35 -0500, Les Cargill wrote:
>
>> Tim Wescott wrote:
>>> This is related to my question about interrupts in an STM32F303
>>> processor. It turns out that the problem is in the compiler (or I'm
>>> going insane, which is never outside the realm of possibility when I'm
>>> working on embedded software).
>>>
>>> I'm coding in C++, and I'm using a clever dodge for protecting chunks
>>> of code from getting interrupted. Basically, I have a class that
>>> protects a block of code from being interrupted. The constructor saves
>>> the interrupt state then disables interrupts, and the destructor
>>> restores interrupts.
>>>
>>> This has been reliable for me for years, but now the destructor is not
>>> being called. I suspect that the optimizer can't make sense of it
>>> because of the asm statements, and is throwing it away.
>>>
>>> If someone knows the proper gnu-magic to tell the optimizer not to do
>>> that, I'd appreciate it. I'm going to look in my documentation, but I
>>> want to make sure I use the right method, and don't just stumble onto
>>> something that works for now but should be depreciated, or is fragile,
>>> or whatever.
>>>
>>> Here's the "protect a block" class:
>>>
>>> typedef class CProtect {
>>> public:
>>>
>>> CProtect(void)
>>> {
>>> int primask_copy;
>>> asm("mrs %[primask_copy], primask\n\t" // save interrupt
>>> status
>>> "cpsid i\n\t" // disable interrupts :
>>> [primask_copy] "=r" (primask_copy));
>>> _primask = primask_copy;
>>> }
>>>
>>> ~CProtect()
>>> {
>>> int primask_copy = _primask;
>>> // Restore interrupts to their previous value asm("msr primask,
>>> %[primask_copy]" : : [primask_copy]
>>> "r" (primask_copy));
>>> }
>>>
>>> private:
>>> volatile int _primask;
>>> } CProtect;
>>>
>>> and here's how it's used:
>>>
>>> {
>>> CProtect protect;
>>>
>>> // critical code goes here
>>> }
>>>
>>>
>>
>> There are no side effects in the destructor.
>
> Yes there are. the MSR instruction loads the contents of a register
> (primask) into the mask register, setting the mask bit to whatever's in
> the register's bit 0.
>
I do not think the compiler understands side effects in asm code. SFAIK,
those are opaque to the optimizer.
If that were not true, it would not optimize the asm stuff out.
>> Notes:
>> - extern is not extern "C". You can dern tootin' make 'em
>> extern "C" but I don't see the point.
>> - SFAIK, the "extern...inline" combination works. I have
>> no way to test it. "extern...inline' isn't critical; it's
>> just more or less an apology for making things slightly more
>> complex.
>> - If you are not using separate compilation it gets
>> even simpler.
>>
>> I would:
>>
>> #ifndef _CPROTECT_H_
>> #define _CPROTECT_H_
>> // CProtect.h
>>
>> extern void inline interruptsOn(void); extern void inline
>> interruptsOff(void);
>>
>> typedef class CProtect {
>> private:
>>
>> public:
>>
>> CProtect(void)
>> {
>> interruptsOff();
>> }
>>
>> ~CProtect()
>> {
>> interruptsOn();
>> }
>>
>> } CProtect;
>> #endif
>>
>>
>>
>> // CProtect.cpp
>>
>> #include <CProtect.h>
>>
>> static int _primask;
>>
>> void inline interruptsOff(void)
>> {
>> int primask_copy;
>> asm("mrs %[primask_copy], primask\n\t" // save interrupt status
>> "cpsid i\n\t" // disable interrupts :
>> [primask_copy] "=r" (primask_copy));
>> _primask = primask_copy;
>> }
>>
>> void inline interruptsOn()
>> {
>> int primask_copy = _primask;
>> // Restore interrupts to their previous value asm("msr primask,
>> %[primask_copy]" : : [primask_copy]
>> "r" (primask_copy));
>> }
>
> Yes, very good. And the first time you hit the block with interrupts
> OFF, they will be (erroneously) ON when you exit it.
>
interruptsOn() is only called by the destructor. We presume the
destructor is only called at the end of a block where a
CProtect object has been instantiated.
> Hence, you want to save the interrupt state and restore it on exit.
>
I believe what I put up there is equivalent to
what you'd put up. I just added some scaffolding to tweak
some of the framing rules in C++.
If I made some mistake in that way, my bad - I have just
used this technique with GNU before.
--
Les Cargill
Reply by Les Cargill●May 31, 20152015-05-31
Dave Nadler wrote:
> On Thursday, May 28, 2015 at 10:17:18 PM UTC-4, Les Cargill wrote:
>> ...
>> CProtect(void)
>> {
>> interruptsOff();
>> }
>
> Careful Les, that's a bug if entered in a context where
> interrupts are already disabled and must remain so...
>
"Doctor, doctor is hurts when I do that..."
That is inherent in the design of this widget.
--
Les Cargill
Reply by Les Cargill●May 31, 20152015-05-31
David Brown wrote:
> On 29/05/15 04:23, Les Cargill wrote:
>> Tim Wescott wrote:
>>> This is related to my question about interrupts in an STM32F303
>>> processor. It turns out that the problem is in the compiler (or I'm
>>> going insane, which is never outside the realm of possibility when I'm
>>> working on embedded software).
>>>
>>> I'm coding in C++, and I'm using a clever dodge for protecting chunks of
>>> code from getting interrupted. Basically, I have a class that protects a
>>> block of code from being interrupted. The constructor saves the
>>> interrupt state then disables interrupts, and the destructor restores
>>> interrupts.
>>>
>>> This has been reliable for me for years, but now the destructor is not
>>> being called. I suspect that the optimizer can't make sense of it
>>> because of the asm statements, and is throwing it away.
>>>
>>> If someone knows the proper gnu-magic to tell the optimizer not to do
>>> that, I'd appreciate it. I'm going to look in my documentation, but I
>>> want to make sure I use the right method, and don't just stumble onto
>>> something that works for now but should be depreciated, or is fragile, or
>>> whatever.
>>>
>>> Here's the "protect a block" class:
>>>
>>> typedef class CProtect
>>> {
>>> public:
>>>
>>> CProtect(void)
>>> {
>>> int primask_copy;
>>> asm("mrs %[primask_copy], primask\n\t" // save interrupt status
>>> "cpsid i\n\t" // disable interrupts
>>> : [primask_copy] "=r" (primask_copy));
>>> _primask = primask_copy;
>>> }
>>>
>>> ~CProtect()
>>> {
>>> int primask_copy = _primask;
>>> // Restore interrupts to their previous value
>>> asm("msr primask, %[primask_copy]" : : [primask_copy]
>>> "r" (primask_copy));
>>> }
>>>
>>> private:
>>> volatile int _primask;
>>> } CProtect;
>>>
>>> and here's how it's used:
>>>
>>> {
>>> CProtect protect;
>>>
>>> // critical code goes here
>>> }
>>>
>>
>>
>> There are no side effects in the destructor.
>>
>> Notes:
>> - extern is not extern "C". You can dern tootin' make 'em
>> extern "C" but I don't see the point.
>> - SFAIK, the "extern...inline" combination works. I have
>> no way to test it. "extern...inline' isn't critical; it's
>> just more or less an apology for making things slightly
>> more complex.
>> - If you are not using separate compilation it gets
>> even simpler.
>>
>> I would:
>>
>> #ifndef _CPROTECT_H_
>> #define _CPROTECT_H_
>> // CProtect.h
>>
>> extern void inline interruptsOn(void);
>> extern void inline interruptsOff(void);
>>
>> typedef class CProtect
>> {
>> private:
>>
>> public:
>>
>> CProtect(void)
>> {
>> interruptsOff();
>> }
>>
>> ~CProtect()
>> {
>> interruptsOn();
>> }
>>
>> } CProtect;
>> #endif
>>
>>
>>
>> // CProtect.cpp
>>
>> #include <CProtect.h>
>>
>> static int _primask;
>>
>> void inline interruptsOff(void)
>> {
>> int primask_copy;
>> asm("mrs %[primask_copy], primask\n\t" // save interrupt status
>> "cpsid i\n\t" // disable interrupts
>> : [primask_copy] "=r" (primask_copy));
>> _primask = primask_copy;
>> }
>>
>> void inline interruptsOn()
>> {
>> int primask_copy = _primask;
>> // Restore interrupts to their previous value
>> asm("msr primask, %[primask_copy]" : : [primask_copy]
>> "r" (primask_copy));
>> }
>>
>
> This looks like you believe making an external function call enforces a
> memory order. This is simply incorrect - if the compiler knows the
> effects the function may have, it can use that information to re-order
> the code. In particular, it knows something about the data the function
> could access (it cannot legally access function-local or file static
> data if the data does not "escape" by passing pointers to it out to
> unknown code), and it may have full access to the source code through
> inlining or link-time optimisation.
>
It's less that than that _primask is now a side effect. Again,
I can't test this in his environment so the idea is to suggest
a possible direction less than a destination.
It may well be that interruptsOn() doesn't quite work
unless you write back to _primask. Or something.
Others have more directly addressed direct directives to tell the
optimizer to hold off. I also understand not wanting to do that.
> So while the use of external functions might appear to work, and /will/
> work in some contexts, it will work by accident of compiler flags and
> limited compiler capabilities, rather than by design. The use of
> "inline" (or "extern inline") here is utterly pointless
Of course.
> - if the
> compiler is able to inline the code, then you lose all hope of it
> working.
Not so much. It'll work either way.
> And when the code is /not/ inlined, you generate significantly
> bigger and slower code for something that should boil down to 2
> instructions for the constructor and 1 instruction for the destructor.
>
Life is like that. I would have used 'C' macros myself
because those always work, but the use of a class to do this
is pretty slick.
>
>
--
Les Cargill
Reply by Tim Wescott●May 29, 20152015-05-29
On Sat, 30 May 2015 01:57:47 +0200, David Brown wrote:
> On 29/05/15 22:37, DJ Delorie wrote:
>>
>> David Brown <david.brown@hesbynett.no> writes:
>>> The msp430 gcc port has an elegant method - it implements a "critical"
>>> function attribute, which causes interrupt state to be saved and
>>> disabled at the start of the function, and restored at the end. It
>>> works fine with inline functions too. I would like to see that one
>>> implemented on other gcc targets (if you have the spare time, DJ ?).
>>
>> That's a highly target-specific attribute, so each target maintainer
>> would have to add it separately. I think some other targets have
>> similar features, though, but it's not something standard across all
>> targets.
>>
>>
> I know that it's a target-specific attribute, but in theory it could be
> made common across a range of targets (at least most typical embedded
> microcontrollers). If there were common functions such as
> __builtin_disable_interrupts returning the current interrupt mask, and
> __builtin_restore_interrupt_mask to restore it, you could make those
> target-specific while the "critical" attribute could be common to every
> target that implemented those builtins.
>
> For those of us who work with a variety of gcc microcontroller targets,
> it can be frustrating to see useful extensions from one target
> unavailable on other targets. As well as "critical", another that I
> miss on some targets is "naked".
Part of the reason for the "CProtect" class is to allow me to wrap
different things in the same syntax. It may not be portable outside of
Wescott Design Services, but it means that my internal code can be
portable across different processors.
--
Tim Wescott
Wescott Design Services
http://www.wescottdesign.com