5
\$\begingroup\$

I'm using the following STM32's HAL function to read a 12-bit ADC value (0 to 4095) from a channel:

__weak uint32_t HAL_ADC_GetValue(ADC_HandleTypeDef* hadc)
  {
  /* Note: This function is defined into this file for library reference. */
  /* Function content is located into file stm32f3xx_hal_adc_ex.c   */
      
  /* Return ADC converted value */ 
  return hadc->Instance->DR;    
}

As you see, the function above returns a 32-bit unsigned integer.

At the start of my code I declare the grabbed ADC value as volatile uint16_t adc_val.

At some point I call the HAL functions and assign it to the adc_val as follows:

HAL_ADC_Start_IT(&hadc1);
if (HAL_ADC_PollForConversion(&hadc1, 5) == HAL_OK) {
  adc_val = HAL_ADC_GetValue(&hadc1);
}
HAL_ADC_Stop(&hadc1);

Now HAL_ADC_GetValue returns 32-bit unsigned but adc_val is declared as 16-bit unsigned.

My question is, do we need casting here? And what happens if we keep it like in my code without casting? (The reason I want to use 16-bit unsigned is to make the code faster since 16-bit unsigned is sufficient for a 12-bit ADC)

\$\endgroup\$
20
  • 3
    \$\begingroup\$ Casting will just keep the compiler from maybe issuing a warning. The compiler will just truncate the top 16 bits. Why do you think 16 bit operations will be faster on a 32bit machine? It’s more likely to be slower as extra code gets added to manage 16 bits. It would be slower on an 8 or 16 bit machine. If in doubt do some benchmarking. \$\endgroup\$
    – Kartman
    Oct 6, 2022 at 9:52
  • 1
    \$\begingroup\$ I don't know a thing about the STM32 architecture. If this were a DEC Alpha I'd say you are pretty much stuck with masking and shifting things around and the associated price you'd pay. They absolutely refused to include any lane-shifting code in that architecture. But in contrast most other architectures do include some means of lane-shifting and masking as part of their internal structures. To write C code that uses an architecture well requires a study of that architecture and an understanding of the specific C compiler you are stuck using on it. All of that matters a lot. \$\endgroup\$
    – jonk
    Oct 6, 2022 at 9:57
  • 1
    \$\begingroup\$ @jonathanjo Zero built-in lane shifting. I'm talking about inside the proc. Not outside assembly instructions, which I don't care about. If you see the architecture itself you can see that it is 64-bit lanes and NO lane-shifting to be seen. Of course, people want lane-shifting and there are means by which to get it. But not intrinsically within the Alpha hardware design. I was there. It was probably the most RISC processor I've ever seen. They went crazy hog-wild RISC on this one. If you care, just dig into the pipeline and exception event handling. That will fry your brain. \$\endgroup\$
    – jonk
    Oct 6, 2022 at 10:49
  • 1
    \$\begingroup\$ @jonathanjo It was in the early 1990's -- can't recall exactly but I think I got into it around 1993 or so? Anyway, no time for chatting. It's literally bedtime for me and I should have already gone to bed and started playing Return to Monkey Island or something to go to sleep! There's plenty in the docs on the exception issues -- it's what literally caused me to stop any ideas of working with the Alpha! Restarting from that mess drove me bat-crazy! I'm sure the documentation authors also went crazy writing it! They froze the entire pipeline and sometimes allowed a restart from it!!! \$\endgroup\$
    – jonk
    Oct 6, 2022 at 11:01
  • 3
    \$\begingroup\$ @jonathanjo - I think jonk is just trying to explain the fact that early Alpha had no unaligned loads, and not byte or halfword loads, only aligned 32 and 64-bit loads. That let them avoid some muxing/decoding in the load/store hardware, and make the ECC granules for their L1d cache be 32-bit instead of 8-bit for much lower overhead, without ever needing an internal RMW cycle to update the containing word's ECC data. (So pipelining is easier). It's the one modern CPU where Can modern x86 hardware not store a single byte to memory? is actually true. \$\endgroup\$ Oct 6, 2022 at 19:28

5 Answers 5

24
\$\begingroup\$

(The reason I want to use 16-bit unsigned is to make the code faster since 16-bit unsigned is sufficient for 12-bit ADC)

Why would 16 bits be faster than 32 bits on a 32 bit MCU? Your rationale for messing around with this to begin with doesn't make much sense. If you used 16 bits to save 2 bytes of RAM, that would have been a valid rationale.


make the code faster

Declaring variables as volatile just for the heck of it, is an excellent way of making your program much slower. Stop making your program slower if you want it to be faster...

The only thing that must be volatile here is the actual register declaration.

In general, it isn't wise to attempt manual code optimization unless you have in-depth knowledge of C and how C code translates to machine code for the given target.


My question is, do we need casting here?

No. The only relevant quote here is from the C standard ISO 9899:2018 6.3.1.3 regarding unsigned/signed integer conversions (normative text):

Otherwise, if the new type is unsigned, the value is converted by repeatedly adding or subtracting one more than the maximum value that can be represented in the new type until the value is in the range of the new type.60)

Where further info is given by the foot note 60 (informative text):

  1. The rules describe arithmetic on the mathematical value, not the value of a given type of expression.

In theory, the above says that you'd take the uint32_t value, write it down with pen & paper and repeatedly subtract UINT16_MAX + 1 from it, until you get in range with what would fit inside a uint16_t

Which as it happens is the same as taking the uint32_t value modulus the maximum for the uint16_t. Which as it happens is the same as simply discarding the most significant bytes.

That being said, it is good practice to never have any implicit conversions in your code. Standards like MISRA C disallow such conversions, as one example. So it isn't wrong to add a manual cast as a way of self-documenting code or adding one for MISRA compliance. But in this specific case, it doesn't change anything in practice.

\$\endgroup\$
12
  • 2
    \$\begingroup\$ Upvoted. Your comments about volatile and whether 16-bit is faster/slower are excellent, and very helpful quotation and reference to the portion of the current ISO standard. I disagree about "The only relevant quote". The question doesn't say what C compiler is in use, what strictness flags might be in use, or which standards (if any) are applicable in that project. GCC's position is "GCC attempts to follow one or more versions of that standard", and for example gcc has at least half a dozen flags for various C standards, but the default isn't your "only relevant" standard. \$\endgroup\$
    – jonathanjo
    Oct 6, 2022 at 16:04
  • 2
    \$\begingroup\$ @jonathanjo As for gcc & others, they seek to provide backwards compatibility for the sole purpose of maintaining old programs. Depending on which version you have, gcc only presents two standards: std=gnu17, which is "relaxed standard C with extensions" and std=c17 "relaxed standard C", which is ISO 9899:2018 but not to the letter. You can also force it to become a conforming compiler with -std=c17 -pedantic. \$\endgroup\$
    – Lundin
    Oct 7, 2022 at 6:44
  • 1
    \$\begingroup\$ I respect the opinion that there's "only one standard", but it's somewhat of a theoretical ideal. For example it doesn't cover maintenance well. We don't recompile and redeploy our projects because a standards organisation has issued a new standard. And in every serious project I've worked on, we make a conscious decision about what standards to adopt, and under what conditions those standards will change. When we do maintenance on old projects (in my organisations) we usually do it with toolchains which we have kept operational for this purpose. \$\endgroup\$
    – jonathanjo
    Oct 7, 2022 at 6:46
  • 4
    \$\begingroup\$ @ScottSeidman Assuming you mean typedef union { uint16_t u16 [2]; uint32_t u32} my_adc_t; then probably not. This union will have the same endianess as the CPU. However, ADCs do not necessarily follow CPU endianess (especially not when external) and they almost always have the option to left/right "align" (shift) data as well. Meaning that some 12 bit ADC with a 16 bit result register, making a reading of value 0x123, might present it either as 0x0123 or 0x2301 or 0x1230 or 0x3012... or 32-bit flavours of those. \$\endgroup\$
    – Lundin
    Oct 7, 2022 at 11:37
  • 4
    \$\begingroup\$ I didn't read all of the comments (WOW, this one is popular!), but I wanted to point out another potential pitfall: Which way does this HAL library justify the ADC result? If it's right-justified, then a simple cast is fine, albeit unnecessary. If it's left-justified, then you could lose everything and end up with all zeros. (one reason to left-justify is to present the result as a fraction of the reference, using fixed-point format 0.x, which can be a useful step in keeping a relevant set of units) \$\endgroup\$
    – AaronD
    Oct 7, 2022 at 13:44
5
\$\begingroup\$

Updated after discussion

Harbison and Steele (and every version of every standard I've looked at) say no cast is required, and that the implicit conversion simply throws away the high-order bits.

If the destination type is shorter than the source type and both the original and destination types are unsigned, then the conversion can be performed simply by discarding excess high-order bits from the original value. The bit pattern of the result respresentation will be equal to the n low-order bits of the original respresentation, where n is the number of bits in the destination type.
-- C: A Reference Manual (5th ed) section 6.2.3

If you have uncertainties about what the implicit conversations are, it's well worth reading up on them, and it is always worth finding out exactly what your compiler does for the flags and options you're using.

\$\endgroup\$
1
  • \$\begingroup\$ Comments are not for extended discussion; this conversation has been moved to chat. \$\endgroup\$
    – Voltage Spike
    Oct 7, 2022 at 14:11
2
\$\begingroup\$

That is perfectly valid code. You don't need to do anything, no casting required.

\$\endgroup\$
2
\$\begingroup\$

As the others above say, there is no need for casting.

HOWEVER, it is a good idea to cast it so that it is clear to later engineers exactly what is happening.

\$\endgroup\$
3
  • 6
    \$\begingroup\$ There is an alternative view (which I generally subscribe to): never use a cast if there is another way to do it, because it basically switches off a lot of checking. In this particular case, I'd normally suggest something like shortvar = longfunc() & 0xffff; which not only deals with the sizes but explicitely says what we think should happen. Indeed, we know it's 12 bits: shortvar = longfunc() & 0xfff; \$\endgroup\$
    – jonathanjo
    Oct 6, 2022 at 11:36
  • 6
    \$\begingroup\$ @jonathanjo A better view to subscribe to is to never allow any implicit conversions anywhere in your code and never write any code relying on them either. Your examples here look like they come with 2 implicit conversions, can you point them out and explain them? Do you actually know the type of 0xffff on a generic computer and why the compiler picked it? And so on. Programmers that don't know/care about implicit promotions are the kind who end up with mysterious bugs and then after a long while of debugging come up with desperate "just cast here and it works" without even knowing why. \$\endgroup\$
    – Lundin
    Oct 6, 2022 at 15:12
  • 2
    \$\begingroup\$ @Lundin We appear to be agreeing that many defensive programming techniques are useful, including avoiding implicit conversions. Which ones are considered acceptable depends on a lot of things, not least of which is the intended audience, longevity, and deployment of any particular piece of code. \$\endgroup\$
    – jonathanjo
    Oct 6, 2022 at 15:40
2
\$\begingroup\$

The rational about using a 16-bit wide type aside:

It's often times a good idea to use explicit type conversions (aka casts), if you are aware of a potential dangerous type conversion (i. e. potential loss of information) and want to indicate your awareness to the reader of your code and/or a static analysis tool.

So in this case, the type conversion of a 32-bit type to a 16-bit type might cause loss of information. But if you know that HAL_ADC_GetValue() will always return only 12 bit of information, you should use a cast. Or to be more precise: If you know that the return value of HAL_ADC_GetValue() will not lose information when it is assigned to a 16-bit type. It's not necessary syntactically, of course.

However, I'm not a fan of avoiding implicit type conversions altogether. Many implicit type conversions don't do any harm. Just set up a static analyzer to flag all implicit conversions and see what happens. And then look at your code after turning all implicit conversions to explicit ones. Not good. You not only bloated your code, you also turned explicit type conversions into a brain-dead activity.

(The reason I want to use 16-bit unsigned is to make the code faster since 16-bit unsigned is sufficient for a 12-bit ADC)

If you care about speed, you could use the fast-types of stdint.h. But if you have a look in your stdint.h, you'll most likely find that uint_fast16_t is typedef'd to unsigned int anyway. Plus you kind of obfuscate the actual width of the data type.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.