EPICS Controls Argonne National Laboratory

Experimental Physics and
Industrial Control System

2002  2003  2004  2005  2006  2007  2008  2009  2010  <20112012  2013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024  Index 2002  2003  2004  2005  2006  2007  2008  2009  2010  <20112012  2013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
<== Date ==> <== Thread ==>

Subject: Re: [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base
From: Jeff Hill <[email protected]>
To: Jeff Hill <[email protected]>
Date: Fri, 19 Aug 2011 22:15:23 -0000
Responding to Michael's comments:

> 1) How about add and subtract?  For instructions sets which don't support this directly 
> it can be implemented in terms of compare+swap.*

That interface would probably be best implemented with signed, and not unsigned, integer types. My current position (subject to change :-) is that I don't have a need for this but if you do then any such features should not delay the initial merge unless you do have immediate needs. I have a large amounts of code to be merged, and all of it is potentially blocked while this merge is pending. Also, there is daily code maintenance merging in changes from the main trunk while this merge is pending.

> 2) I would suggest not making the locked functions part of the public API.  I don't see one 
> piece of code needing to use both epicsLockedIncrSizeT and epicsAtomicIncrSizeT.  Perhaps 
> instead have a macro to force a fallback to the default implementation?

The original reasons for the epicsLockedXxxx public interface was so that we _could_ have macros forwarding the epicsAtomixXxxx to epicsLockedXxxx. That requires a visible function prototype somewhere for the epicsLockedXxxx interface. However, it turns out that independently I recently eliminated the epicsLockedXxxx interface (see revision 12248) because I decided that it was simpler and more efficient to just implement the locked version in OS specific code. So I didn't like the epicsLockedXxxx functions polluting the public interface either, and they have now been eliminated.

> 3) As a thought, how about signed instead of unsigned integers?  This 
> would differentiate 'int' from 'size_t' operations even on 32-bit machines.

I implemented the unsigned set/get/compare-and-swap interface for compare-and-swap applications where one is manipulating enumerated states.

Personally, I very rarely use signed integers in my code because I don't like the overhead of sanity checking both sides of the range. So that hopefully explains how I ended up with size_t and unsigned which are admittedly the same on 32 bits.

So another option would be to implement a "unsigned short" interface which would be arguably distinct from size_t on _all_ architectures, would handle arguably a sufficient range of compare-and-swap enumerated flags, and would involve less storage penalty. You would loose the negative range for compare-and-swap enumerated flags compared to "signed short" but does anyone really need a negative range for compare-and-swap enumerated flags? From my perspective adding that negative range only opens up opportunities for bugs resulting from a negative out of range array index. Of course its a bad idea to mix integers and unsigned integers together in arithmetic expressions so if you start with unsigned enumerated flags you have to stay consistent. Admittedly, the K&R C guys made the maybe bad choice to make C enum type be always negative range capable, but that's unchangeable and this _is_ therefore an argument in favor of using instead "signed short" (there I go arguing against my!
 self again :-). However note that enum is the same size as int so there would have to be a range check before converting enum to signed or unsigned short (that's an argument for your original suggestion of an int). So let me know what your consensus is and I will implement it - one of {int, unsigned short, or signed short} for the set/get/compare-and-swap integer interface. I will vote for "unsigned short" based on my needs  (I can use "static const unsigned short flagXxx = nnn" instead of enum for my flag values).

I do agree that if we implement an add and subtract interfaces then that _should_ use signed integers.


-- 
https://code.launchpad.net/~epics-core/epics-base/epicsR3.15-atomics/+merge/70642
Your team EPICS Core Developers is requested to review the proposed merge of lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base.


References:
[Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base Jeff Hill

Navigate by Date:
Prev: Re: [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base Jeff Hill
Next: [Merge] lp:~anj/epics-base/epicsEvent-api into lp:epics-base noreply
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  <20112012  2013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
Navigate by Thread:
Prev: Re: [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base Jeff Hill
Next: Re: [Merge] lp:~epics-core/epics-base/epicsR3.15-atomics into lp:epics-base Jeff Hill
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  <20112012  2013  2014  2015  2016  2017  2018  2019  2020  2021  2022  2023  2024 
ANJ, 02 Feb 2012 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· Search · EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·