Summator simulation

This is a reference implementation of a summation unit. The algorithm used is a most straightforward carry-propagation. If necessary, a test driving code could be provided.

Few notes for the reviewers:

• The code does compile and works indeed. Test harness is available on demand.

• I am a steadfast proponent of descriptive names. This is a rare case where one-letter names are descriptive. N,Z,C,V were adopted and blessed by generations of designers. Calling the flag overflow instead of V would sound like a blasphemy.

• Validity of register indices is intentionally not verified. Just pretend that they are always valid.

• For a V calculation, refer to this wonderful explanation

Looking for an Ultimate Review of the Clarity, the Universe and Everything.

```
#include
#include
#include
#include

template
struct GPR {
std::vector bits;

GPR(): bits(std::numeric_limits::digits, false) {};
~GPR() { delete bits; }

bool sign() const { return bits.back(); }

auto begin() -> decltype(bits.begin()) { return bits.begin(); }
auto end() -> decltype(bits.end()) { return bits.end(); }
auto rbegin() -> decltype(bits.rbegin()) { return bits.rbegin(); }
auto rend() -> decltype(bits.rend()) { return bits.rend(); }

void load(Int value) {
for(auto it = begin(); it != end(); ++it) {
*it = value & 0x01;
value >>= 1;
}
}

void store(Int& value) {
for(auto it = rbegin(); it != rend(); ++it) {
value
class CPU {
struct flags {
bool N;
bool C;
bool Z;
bool V;
flags(): N(false), C(false), Z(false), V(false) {}
friend std::ostream& operator > regfile;

void add_internal(GPR& r1, GPR& r2) {
bool s1 = r1.sign();
bool s2 = r2.sign();
flags.Z = true;
std::transform(r1.begin(), r1.end(), r2.begin(), r1.begin(), adder(flags));
flags.N = r1.sign();
flags.V = !(s1 ^ s2

First of all I would like to dispute your assertion that the one letter names are descriptive. The only people how will find them descriptive are those who are used to them but readers that have no understanding of the topic are at a total loss here (not having any comments on the variables reinforces this).

But this is only my opinion.

On to more substantial problems:

Using vector considered harmful?

I see that you are using the dreaded vector. I also see that you don't change its size after initialization and that the size is known at compiletime. Together with the fact that you are doing bit manipulations this calls for a std::bitset.

However, fact that vector offers these nice iterators that you can use with the standard algorithms might be enough of a reason to use it nonetheless.

delete std::vector()?

Why are you deleting a non pointer in GPR's destructor:

std::vector bits;
//...
~GPR() { delete bits; }

I am nearly certain that this should be a compiler error (although my compiler only warns about it)

C++11

Why didn't you use a range based for in GPR::load?

for(auto ¤tBit: bits) {
    currentBit = value & 0x01;
    value >>= 1;
}

Using boost's reversed range adaptor you could also do this in store.

Compiler warnings about operator order

Compiling your code gave me warnings about recommended parentheses on:

f.C = x & y | f.C & (x ^ y);

and

flags.V = !(s1 ^ s2) & (s1 ^ r1.sign());

Output order of the bits of reg

You output the bits in reg with the least significant coming first and the sign last. Is this intended?

std::copy(reg.begin(), reg.end(), os);

Or should it be

std::copy(reg.rbegin(), reg.rend(), os);

And while we are at it you surely mean std::ostream_iterator(os) instead of os.

Flag Output

The output for the flags is only understandable when you read the code and see the order. At the very least I would output the flag "descriptor" instead of 0 or 1 to better indicate which flags are set.

You could use the case to identify the flag or simply leave out those that are not set.

Extract non template members

Neither flags nor adder depend on the template parameter of CPU so you should take it out of the that class to avoid code bloat due to unnecessary multiple instantiations.

If you insist on making them private members than you should put them in a non templated base class for CPU.

Naming

• the member variable CPU::flags shadows the member struct CPU::flags

• adder -> BitAdder (or bit_adder but I would choose the former to distinguish class and function names)

• regfile -> registers (although the former might be more domain specific)

StackExchange Code Review Q#61924, answer score: 6