memcpy not optimised out during attempt at ‘fast’ pimpl

Question

I need to use a very large and complex header-only class (think boost::multiprecision::cpp_bin_float<76>, called BHP below) which I would like to hide behind a pimpl-like implementation, purely to reduce compilation time in a somewhat large project (replacing the Boost class with std::complex<double> reduced compilation times by approx. 50%).

However, I would like to avoid dynamic memory allocations. Hence, something like this seems natural (ignoring alignment issues for now which can be avoided using aligned_storage or alignas):

struct Hidden {
  char data[sz];

  Hidden& punned(Hidden const& other);
};

Hidden::punned can then be defined in a single translation unit to cast data to BHP*, act on it and not pollute all other translation units with 170k LOC of header files. A possible implementation might be

Hidden& Hidden::punned(Hidden const& other) {
  *(BHP*)(data) += *(BHP*)(other.data);
  return *this;
}

This, of course, is undefined behaviour, because we access an object of type BHP through a pointer of type char, thus violating strict aliasing rules. The proper way to do this is:

Hidden& Hidden::proper(Hidden const& other) {
  BHP tmp; std::memcpy(&tmp, data, sz);
  BHP tmp2; std::memcpy(&tmp2, other.data, sz);
  tmp += tmp2;
  std::memcpy(data, &tmp, sz);
  return *this;
}

Now it might appear ‘obvious’ that these memcpy calls could be optimised out. Unfortunately, this is not the case, they remain and make proper() much larger than punned().

I would like to know what the correct way to a) store the data directly in the Hidden object and b) avoid unnecessary copies to re-interpret it and c) avoid violations of the strict alignment rule and d) do not carry around an extra pointer pointing at the storage area.

There is a godbolt link here; note that all compilers I tested (GCC 4.9 - trunk, Clang 3.9, 4.0 and 5.0 and Intel 18) did not ‘optimise out’ the memcpy. Some versions of GCC (e.g. 5.3) also outright complain about a violation of the strict aliasing rule, though not all of them do. I’ve also inserted a Direct class which knows about BHP and hence can call it directly, but I would like to avoid this.

Minimal working example:

#include <cstring>

constexpr std::size_t sz = 64;

struct Base {
  char foo[sz];
  Base& operator+=(Base const& other) { foo[0] += other.foo[0]; return *this; }
};
typedef Base BHP;

// or:
//#include <boost/multiprecision/cpp_bin_float.hpp>
//typedef boost::multiprecision::number<boost::multiprecision::cpp_bin_float<76> > BHP;

struct Hidden {
  char data[sz];

  Hidden& proper(Hidden const& other);
  Hidden& punned(Hidden const& other);
};

Hidden& Hidden::proper(Hidden const& other) {
  BHP tmp; std::memcpy(&tmp, data, sz);
  BHP tmp2; std::memcpy(&tmp2, other.data, sz);
  tmp += tmp2;
  std::memcpy(data, &tmp, sz);
  return *this;
}

Hidden& Hidden::punned(Hidden const& other) {
  *(BHP*)(data) += *(BHP*)(other.data);
  return *this;
}

struct Direct {
  BHP member;
  Direct& direct(Direct const& other);
};

Direct& Direct::direct(Direct const& other) {
  member += other.member;
  return *this;
}

struct Pointer {
  char storage[sz];
  BHP* data;

  Pointer& also_ok(Pointer const& other);
};

Pointer& Pointer::also_ok(Pointer const& other) {
  *data += *other.data;
  return *this;
}

_"This, of course, is undefined behaviour, because we access an object of type BHP through a pointer of type char, thus violating strict aliasing rules."_ No, converting some other pointer type to `char*` and reinterpreting through the latter is exempt from aliasing. It *has* to be, for it to be possible to implement `memcpy()` (without compiler tricks) and the process of 'bit-casting' using it. However, the converse is not true. — underscore_d, Dec 14 '17 at 14:39
@underscore_d I don't understand what you mean by "reinterpreting through the latter"? — Joe, Dec 14 '17 at 14:43
@Joe dereferencing the cast `char*` & using it to read the object representation of an object, i.e. reinterpreting the object as bytes. That's allowed. Casting something that is just a set of bytes to some other type, which it wasn't constructed as, is not. In other words, the pointer obtained from a cast changing the type can only be dereferenced if there's really an object of that type (or a compatible one) there; we can't just create an array of `char`s, fill it with data & tell the compiler _'Here: have a `SomeClass`'_. But we **can** say _'Show me the `char`s making up this `SomeClass`.'_ — underscore_d, Dec 14 '17 at 14:45
@underscore_d And casting via the byte representation to some other type that is not the original type is also disallowed, correct? — Joe, Dec 14 '17 at 14:47
That's just another phrasing of what I already said, so yes. Anyway, it's not clear if this is relevant to the OP; I just wanted to make that first correction. The disallowance of casting a set of `char` to some other object type is why the `memcpy()` workaround is used, as that is well-defined, so long as the object is trivially copyable (or at least that was the preferred jargon, last I read; they change this almost every week...) — underscore_d, Dec 14 '17 at 14:49
@underscore_d Are you saying that because I always only keep a `BHP` in the `data` member, casting the `char*` pointer to `data` to `BHP*` is always safe? If so, does this imply that e.g. GCC 5.3 wrongly issues a warning `dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]`? — Claudius, Dec 14 '17 at 14:52
No, I'm saying that casting **to** `char*` is fine, but the converse, casting **from** a set of `char` to some other type, is not. Was that not clear? However, this only applies to reinterpretation/type-punning - in contrast, if there's really a `BHP` there, then both casts are OK: so long as what you finally dereference really points to what you claims it points to, there's no issue that I can see. `[[un]signed] char*` has special exemptions from the rules for aliasing and 'type punning'. — underscore_d, Dec 14 '17 at 14:53
@underscore_d That’s what I was trying to say (we have a char*, we cast it to something else and access the object behind it via this), hence my confusion. — Claudius, Dec 14 '17 at 14:55
Would it perhaps be possible to use *explicit template initialization* for your project? — Martin Ueding, Dec 14 '17 at 15:02
@MartinUeding The problem is that my own project is also mostly header-only, at least at the base. The Boost headers then sneak in everywhere simply by including the base scalar class which is needed for everything building onto it. So this is more of a preprocessor rather than template problem. — Claudius, Dec 14 '17 at 15:09

score 1 · Accepted Answer · answered Dec 14 '17 at 14:52

This, of course, is undefined behaviour, because we access an object of type BHP through a pointer of type char.

That's actually not the case. Accessing through a char* is fine provided that there is actually a BHP object there. That is, as long as on both sides you had:

new (data) BHP(...);

then this is perfectly ok:

*(BHP*)(data) += *(BHP*)(other.data);

Just make sure that your char array is also alignas(BHP).

Note that gcc doesn't like when you reinterpret a char[] sometimes, so you can instead choose to use something like std::aligned_storage_t.

Okay, thank you! I was confused by the GCC warning (it triggers on Godbold with GCC 5.4 and I also got 7.2 to emit it at one point, though I’m not sure when or how). But yes, I always keep a `BHP` there, constructed as you said. — Claudius, Dec 14 '17 at 14:54

memcpy not optimised out during attempt at ‘fast’ pimpl

1 Answers1