Does anyone know if the following "naughty" (implementation dependent)
construct would work "correctly" on the ARM Cortex M4, compiled with
arm-gcc-embedded?
typedef union {
uint32_t phase_accumulator_full;
struct {
uint8_t integer_high : 8,
integer_low : 8,
frac_high : 8,
frac_low : 8;
};
} phase_accumulator;
Union of bitfields and larger type on ARM
Started by ●February 23, 2016
Reply by ●February 23, 20162016-02-23
bitrex <bitrex@de.lete.earthlink.net> writes:> Does anyone know if the following "naughty" (implementation dependent) > construct would work "correctly" on the ARM Cortex M4, compiled with > arm-gcc-embedded? > > typedef union { > uint32_t phase_accumulator_full; > struct { > uint8_t integer_high : 8, > integer_low : 8, > frac_high : 8, > frac_low : 8; > > }; > } phase_accumulator;I am not an expert (ask on comp.arch.embedded) but my understanding is that this sort of thing is guaranteed by the EABI (bugs aside). -- John Devereux
Reply by ●February 23, 20162016-02-23
On 23/02/16 20:36, John Devereux wrote:> bitrex <bitrex@de.lete.earthlink.net> writes: > >> Does anyone know if the following "naughty" (implementation dependent) >> construct would work "correctly" on the ARM Cortex M4, compiled with >> arm-gcc-embedded? >> >> typedef union { >> uint32_t phase_accumulator_full; >> struct { >> uint8_t integer_high : 8, >> integer_low : 8, >> frac_high : 8, >> frac_low : 8; >> >> }; >> } phase_accumulator; > > I am not an expert (ask on comp.arch.embedded) but my understanding is > that this sort of thing is guaranteed by the EABI (bugs aside). >I can't even figure out why it would be considered "naughty" at all. The only implementation-dependent aspects I can see are the alignment of the parts (on the ARM, 8-bit, 16-bit and 32-bit types all have "natural" alignments), and the ordering of the bitfields (which is low bits first - guaranteed by the ARM EABI, and thus consistent across compilers).
Reply by ●February 23, 20162016-02-23
On 02/23/2016 04:34 PM, David Brown wrote:> On 23/02/16 20:36, John Devereux wrote: >> bitrex <bitrex@de.lete.earthlink.net> writes: >> >>> Does anyone know if the following "naughty" (implementation dependent) >>> construct would work "correctly" on the ARM Cortex M4, compiled with >>> arm-gcc-embedded? >>> >>> typedef union { >>> uint32_t phase_accumulator_full; >>> struct { >>> uint8_t integer_high : 8, >>> integer_low : 8, >>> frac_high : 8, >>> frac_low : 8; >>> >>> }; >>> } phase_accumulator; >> >> I am not an expert (ask on comp.arch.embedded) but my understanding is >> that this sort of thing is guaranteed by the EABI (bugs aside). >> > > I can't even figure out why it would be considered "naughty" at all. The > only implementation-dependent aspects I can see are the alignment of the > parts (on the ARM, 8-bit, 16-bit and 32-bit types all have "natural" > alignments), and the ordering of the bitfields (which is low bits first > - guaranteed by the ARM EABI, and thus consistent across compilers). > >"Naughty" if one wants to port the code as-is to another architecture, I guess.
Reply by ●February 23, 20162016-02-23
On Tue, 23 Feb 2016 14:17:59 -0500, bitrex wrote:> Does anyone know if the following "naughty" (implementation dependent) > construct would work "correctly" on the ARM Cortex M4, compiled with > arm-gcc-embedded? > > typedef union { > uint32_t phase_accumulator_full; > struct { > uint8_t integer_high : 8, > integer_low : 8, > frac_high : 8, frac_low : 8; > > }; > } phase_accumulator;First: Yes, it'll work as long as you want the least significant byte to be named "integer_high" and the most significant byte to be named "frac_low". Second: You need to name the bitfield. I'll assume you've named it "bits" -- i.e., phase_accumulator.bits.frac_high. Third: It should be just as fast to use: uint32_t phase_accumulator; static inline uint8_t integer_high(uint32_t x) {return (x >> 24) & 0xff;} static inline uint8_t integer_low (uint32_t x) {return (x >> 16) & 0xff;} static inline uint8_t frac_high (uint32_t x) {return (x >> 8) & 0xff;} static inline uint8_t frac_low (uint32_t x) {return (x >> 0) & 0xff;} This will be portable, and integer_high(phase_accumulator) should read as easily (or more so) as phase_accumulator.bits.integer_high. -- Tim Wescott Wescott Design Services http://www.wescottdesign.com
Reply by ●February 23, 20162016-02-23
On 23/02/16 22:45, bitrex wrote:> On 02/23/2016 04:34 PM, David Brown wrote: >> On 23/02/16 20:36, John Devereux wrote: >>> bitrex <bitrex@de.lete.earthlink.net> writes: >>> >>>> Does anyone know if the following "naughty" (implementation dependent) >>>> construct would work "correctly" on the ARM Cortex M4, compiled with >>>> arm-gcc-embedded? >>>> >>>> typedef union { >>>> uint32_t phase_accumulator_full; >>>> struct { >>>> uint8_t integer_high : 8, >>>> integer_low : 8, >>>> frac_high : 8, >>>> frac_low : 8; >>>> >>>> }; >>>> } phase_accumulator; >>> >>> I am not an expert (ask on comp.arch.embedded) but my understanding is >>> that this sort of thing is guaranteed by the EABI (bugs aside). >>> >> >> I can't even figure out why it would be considered "naughty" at all. The >> only implementation-dependent aspects I can see are the alignment of the >> parts (on the ARM, 8-bit, 16-bit and 32-bit types all have "natural" >> alignments), and the ordering of the bitfields (which is low bits first >> - guaranteed by the ARM EABI, and thus consistent across compilers). >> >> > > "Naughty" if one wants to port the code as-is to another architecture, I > guess.I would say "non-portable" rather than "naughty". "Naughty" implies that this might not work in some circumstances, or perhaps depends on undefined behaviour. This is merely non-portable and relies on implementation-dependent behaviour (which happens to be defined by the ARM EABI, and thus should apply to all ARM compilers).
Reply by ●February 23, 20162016-02-23
On 23/02/16 22:53, Tim Wescott wrote:> On Tue, 23 Feb 2016 14:17:59 -0500, bitrex wrote: > >> Does anyone know if the following "naughty" (implementation dependent) >> construct would work "correctly" on the ARM Cortex M4, compiled with >> arm-gcc-embedded? >> >> typedef union { >> uint32_t phase_accumulator_full; >> struct { >> uint8_t integer_high : 8, >> integer_low : 8, >> frac_high : 8, frac_low : 8; >> >> }; >> } phase_accumulator; > > First: > > Yes, it'll work as long as you want the least significant byte to be > named "integer_high" and the most significant byte to be named "frac_low". >True.> Second: > > You need to name the bitfield. I'll assume you've named it "bits" -- > i.e., phase_accumulator.bits.frac_high.Not necessary true, assuming you are using C11 (rather than C++), or gcc or clang, or any other compiler that supports anonymous structs as an extension. Since this is specifically for the ARM, and not meant to be portable, it is perhaps not unreasonable to use an extension that is available the most common tools for that target. But that's a decision for the OP to make.> > Third: > > It should be just as fast to use: > > uint32_t phase_accumulator; > > static inline uint8_t integer_high(uint32_t x) {return (x >> 24) & 0xff;} > static inline uint8_t integer_low (uint32_t x) {return (x >> 16) & 0xff;} > static inline uint8_t frac_high (uint32_t x) {return (x >> 8) & 0xff;} > static inline uint8_t frac_low (uint32_t x) {return (x >> 0) & 0xff;} > > This will be portable, and integer_high(phase_accumulator) should read as > easily (or more so) as phase_accumulator.bits.integer_high. >No, these functions are not a good substitute. First, they do not do the same thing if "phase_accumulator" is used as a volatile - and such bitfield structures are usually used for hardware registers where volatile is relevant. Using the bitfields, accesses to the separate fields will be done as 8-bit accesses - using the functions, they will be 32-bit accesses. Second, your functions give read-only access, not read-write access - another set is needed for writing. Those are messy. static inline uint32_t set_integer_low (uint32_t x, uint8_t y) { return (x & 0xff00ffff) | ((uint32_t) y << 16); } Third, your functions will /not/ be as fast for volatile types, and the write versions may be significantly slower. Fourth, your functions are ugly in use, especially for setting: phase_accumulator = set_integer_low(phase_accumulator, y) (I'm continuing your slight mixup of treating "phase_accumulator" as the instance of the union, rather than the type.) Fifth, you lose one of the major reasons for using bitfields rather than bitmasks. With bitfields, the field is tied tightly to the type, and therefore the instance of the type. Mistakes are compile-time errors, your IDE can quickly give you assistance at filling out the field names, and your debugger can parse the data. With manual shift-and-mask systems you lose all of that. The cost of using bitfields here is that the code is not directly portable to other targets unless they have the same assumptions about bitfield ordering and alignment (and in reality, most targets do match here). IMHO, it's a small price to pay.
Reply by ●February 23, 20162016-02-23
In article <If2zy.23937$Nf2.15642@fx14.iad>, bitrex@de.lete.earthlink.net says...> > Does anyone know if the following "naughty" (implementation dependent) > construct would work "correctly" on the ARM Cortex M4, compiled with > arm-gcc-embedded? > > typedef union { > uint32_t phase_accumulator_full; > struct { > uint8_t integer_high : 8, > integer_low : 8, > frac_high : 8, > frac_low : 8; > > }; > } phase_accumulator;sumg ting does not look correct? Assuming I understand what you're after. struct _phase_acc { union { uint32_t phase_accumulator_full }; union { uint_8 integer_high:8, integer_low:8, frac_high:8, frc_low : 8; }; } phase_accumulator; You can include a condition to test for the second union to determine the endious of the platform to correctly compile the code for the bit field order. The above struct should overlay to generate a single 32 bit image. Jamie
Reply by ●February 23, 20162016-02-23
M Philbrook wrote:> In article <If2zy.23937$Nf2.15642@fx14.iad>, > bitrex@de.lete.earthlink.net says... >> >> Does anyone know if the following "naughty" (implementation dependent) >> construct would work "correctly" on the ARM Cortex M4, compiled with >> arm-gcc-embedded? >> >> typedef union { >> uint32_t phase_accumulator_full; >> struct { >> uint8_t integer_high : 8, >> integer_low : 8, >> frac_high : 8, >> frac_low : 8; >> >> }; >> } phase_accumulator; > > sumg ting does not look correct? Assuming I understand what you're > after. > > struct _phase_acc { > > union { uint32_t phase_accumulator_full };typo..> union { uint_8 integer_high:8,typo..> integer_low:8, > frac_high:8, > frc_low : 8; > }; > } phase_accumulator; > > You can include a condition to test for the second union to > determine the endious of the platform to correctly compile > the code for the bit field order. > > The above struct should overlay to generate a single 32 bit image.No, it won't. You have two 32 bit unions members of a struct, witch gives a 64 bit struct. -- Ian Collins
Reply by ●February 24, 20162016-02-24
On 23.2.16 23:53, Tim Wescott wrote:> On Tue, 23 Feb 2016 14:17:59 -0500, bitrex wrote: > >> Does anyone know if the following "naughty" (implementation dependent) >> construct would work "correctly" on the ARM Cortex M4, compiled with >> arm-gcc-embedded? >> >> typedef union { >> uint32_t phase_accumulator_full; >> struct { >> uint8_t integer_high : 8, >> integer_low : 8, >> frac_high : 8, frac_low : 8; >> >> }; >> } phase_accumulator; > > First: > > Yes, it'll work as long as you want the least significant byte to be > named "integer_high" and the most significant byte to be named "frac_low". > > Second: > > You need to name the bitfield. I'll assume you've named it "bits" -- > i.e., phase_accumulator.bits.frac_high. > > Third: > > It should be just as fast to use: > > uint32_t phase_accumulator; > > static inline uint8_t integer_high(uint32_t x) {return (x >> 24) & 0xff;} > static inline uint8_t integer_low (uint32_t x) {return (x >> 16) & 0xff;} > static inline uint8_t frac_high (uint32_t x) {return (x >> 8) & 0xff;} > static inline uint8_t frac_low (uint32_t x) {return (x >> 0) & 0xff;} > > This will be portable, and integer_high(phase_accumulator) should read as > easily (or more so) as phase_accumulator.bits.integer_high.A vote on this: At least GCC optimizes these to single bit field extract instructions on a Cortex. Alternative format of the same: static inline uint8_t integer_high(uint32_t x) { return (uint8_t)(x >> 24); } This should work correctly even without the cast. -- -TV






