Storing two integer values in 32 bits while avoiding UB with negative numbers

Question

I have two int16_t integers that I want to store in one one 32-bit int.

I can use the lower and upper bits to do this as answered in this question.

The top answers demonstrates this

  int int1 = 345;
  int int2 = 2342;
  take2Integers( int1 | (int2 << 16) );

But my value could also be negative, so will << 16 cause undefined behavior?

If so, what is the solution to storing and retrieving those two numbers which can also be negative in a safe way?

Further context

The whole 32 bit number will be stored in an array with other numbers similar to it.

These numbers will never be utilized as a whole, I'll only want to work with one of the integers packed inside at once.

So what is required is two functions which ensure safe storing and retrieval of two int16_t values into one 4 byte integer

int pack(int16_t lower, int16_t upper);

int16_t get_lower(int);
int16_t get_upper(int);

@user7860670 the final unsigned-to-signed conversion will still be able to overflow. — Quentin, Jun 30 '21 at 13:47
@Quentin You could always `bit_cast`. The round-trip is safe. — Konrad Rudolph, Jun 30 '21 at 13:52
What would you *want* to happen if one number was negative and the other positive? — Adrian Mole, Jun 30 '21 at 13:54
@AdrianMole I simply want to store to numbers which *can* be negative, and retrieve the exact same numbers in a completely safe manner — Lorenzo von matterhorn, Jun 30 '21 at 13:56
@Lorenzovonmatterhorn In C++17 you can use `std::memcpy` instead. But as Adrian correctly notes, once you use `std::memcpy` you can just use it directly for the entire conversion chain, no need for any bit shifting. — Konrad Rudolph, Jun 30 '21 at 13:56
So, you're never going to actually *use* the 32-bit integer? You could have a two-element array of `int16_t` then `std:memcpy` that to/from an `int32_t`. — Adrian Mole, Jun 30 '21 at 13:56
@KonradRudolph Using an array would require quite some refactoring of the project. I wouldn't prefer to do that — Lorenzo von matterhorn, Jun 30 '21 at 14:05
@Lorenzovonmatterhorn Creating an array from two integers is one line of code: `int foo[2] = {a, b};`. You also don’t *need* to use an array, but then you need two `memcpy` calls, and one of them needs to address into the middle of the 32 bit integer, which requires some casts. Using an array is cleaner. — Konrad Rudolph, Jun 30 '21 at 14:07
Any reason you don't use a `struct packed_data{ int16_t low; int16_t high; };`? — NathanOliver, Jun 30 '21 at 14:08
@NathanOliver this would have to be packed to fit into 32 bit. — SergeyA, Jun 30 '21 at 14:09
@SergeyA It is. Of course the OP could use the a packed attribute, but if they plan on being in the x86-64 world there isn't a compiler that wouldn't have this fit in 32 bits. — NathanOliver, Jun 30 '21 at 14:10
@NathanOliver nothing in the question indicates OP is within x86-64. — SergeyA, Jun 30 '21 at 14:16

score 3 · Accepted Answer · answered Jun 30 '21 at 14:10

Here’s the solution with a single std::memcpy from the comments:

std::int32_t pack(std::int16_t l, std::int16_t h) {
    std::int16_t arr[2] = {l, h};
    std::int32_t result;
    std::memcpy(&result, arr, sizeof result);
    return result;
}

Any compiler worth its salt won’t emit code for the temporary array. Case in point, the resulting assembly code on GCC with -O2 looks like this:

pack(short, short):
        sal     esi, 16
        movzx   eax, di
        or      eax, esi
        ret

Without checking, I’m confident that clang/ICC/MSVC produce something similar.

SergeyA · Answer 2 · 2021-06-30T14:14:23.510

1

C++ (pre-20) makes it notoriously hard to alias values. If you want to remain within defined behavior pre-20, you'd have to use memcpy.

Like following:

int32_t pack(int16_t l, int16_t h) {
    int32_t r;
    memcpy(&r, &l, 2);
    memcpy(reinterpret_cast<char*>(&r) + 2, &h, 2);
    
    return r;
}

Unpack is similar, with reverse order of memcpy arguments.

Note, that this code is very-well optimized with both Clang and gcc in my experiments. Here is what was produced by latest version of Clang:

pack(short, short):                              # @pack(short, short)
        movzwl  %di, %eax
        shll    $16, %esi
        orl     %esi, %eax
        retq

It all ended up being the same combination of shifts and conjunctions.

edited Jun 30 '21 at 14:14

answered Jun 30 '21 at 14:06

SergeyA

61,605
5
78
137

This seems like a reasonable solution, is there any additional overhead compared to using shifts? – Lorenzo von matterhorn Jun 30 '21 at 14:09
@Lorenzovonmatterhorn there should be none with any optimizing compiler worth it's upkeep. Will update the answer. – SergeyA Jun 30 '21 at 14:13
I'll port this into the project and run the benchmarks, if I don't see anything suspicious I will accept this answer – Lorenzo von matterhorn Jun 30 '21 at 14:17

Storing two integer values in 32 bits while avoiding UB with negative numbers

2 Answers2