Write, shorten, optimize

• What's inside the assembly code?
• Another way
• Performance measurement
• Wrap-up
• E-books & Webinars

I stumble upon a C++ function that will be a really illustrative example of how refactoring can both shorten and optimize code. Today, we'll need to dust off our human brains in the era of vibe coding and recall what it's like to know what the good is and what the bad is—after all, someone has to do it.

The code snippet below from the vibe-coding project VibeTensor. I like exploring such projects as if I were a naturalist studying a new ecosystem—you can meet with my "field reports" here and there. So, I'm very curious to discover the genesis of new defects and flaws in code. Why? It helps us better understand and describe the challenges code can face in the GenAI era.

One of my discoveries is "code puffiness" that hurts readability and compilers' optimization. The following example showcases it in all its beauty.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  for (std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1; i >= 0; --i) {
    strides[static_cast<std::size_t>(i)] = acc;
    const auto sz = sizes[static_cast<std::size_t>(i)];
    acc *= (sz == 0 ? 1 : sz);
  }

  int64_t ne = 1;
  bool any_zero = false;
  for (auto s : sizes) {
    if (s == 0) {
      any_zero = true;
      break;
    }
    ne *= s;
  }
  if (any_zero) {
    ne = 0;
  }
  ....
}

It may seem that code size and performance are not so critical because they're used in tests.

However, the code is duplicated across nine files, and ideally, that shouldn't happen at all, but if code is going to spread like this, it should at least stay compact.

Also, we shouldn't forget that the generated C++ code must be optimized because the C++ main purpose is to create highly efficient apps. In this case, we have a deal with just the slow test, but in another context, such patterns would significantly downgrade app performance, or all the flawed pieces of generated code that have accumulated over time will have a negative impact.

Slow C++ code goes against the idea of this programming language. That's why it's still a must-have to enhance the expertise in determining whether the code is well-written and how it can be refined, shortened, or optimized. Advanced knowledge can also help in regenerating code with better prompts.

So, let's practice and do some refactoring. Let's start with this code fragment:

int64_t ne = 1;
bool any_zero = false;
for (auto s : sizes) {
  if (s == 0) {
    any_zero = true;
    break;
  }
  ne *= s;
}
if (any_zero) {
  ne = 0;
}

All the array elements are multiplied. If it encounters 0, the loop terminates to avoid unnecessarily processing the remaining array elements because it'll still set the result to zero.

The any_zero flag resets the variable that stores the product, but we can go easier:

int64_t ne = 1;
for (auto s : sizes) {
  if (s == 0) {
    ne = 0;
    break;
  }
  ne *= s;
}

When we encounter 0, ne will be set to 0 and the loop will end. We have another place for simplification: modern processors perform multiplication operations quickly, so we can do the multiplication first and then check it.

int64_t ne = 1;
for (auto s : sizes) {
  ne *= s;
  if (s == 0) {
    break;
  }
}

The code's functionality hasn't changed, but it's become shorter and, in my opinion, even easier to understand.

If we already know that the array is small or that performance isn't critical (for example, because the function is called infrequently), we could simplify it even further:

int64_t ne = 1;
for (auto s : sizes) {
  ne *= s;
}

Let's not think about that, since further actions make the loop be absorbed by another one.

Now let's go back to the beginning of the function.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  for (std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1; i >= 0; --i) {
    strides[static_cast<std::size_t>(i)] = acc;
    const auto sz = sizes[static_cast<std::size_t>(i)];
    acc *= (sz == 0 ? 1 : sz);
  }

The static_cast makes code clunky. The first type casting is needed so that certain compilers or static analyzers don't flag odd arithmetic operations.

std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1;

Let's consider what would happen if we removed the static_cast and the input array turned out to be empty:

• If the array is empty, then nd = 0;
• If we subtract one from an unsigned zero, we get SIZE_MAX, that is, a very large positive unsigned number.
• The SIZE_MAX of thesize_t type is implicitly converted to ptrdiff_t and stored in the i variable. It turns out that i = -1 works as intended. But this is exactly where warnings may be issued. We initialize ptrdiff_t with a number that is larger than the range of the largest number it can hold. Before C++20, the result was implementation-defined. Since C++20, the behavior is well-defined.

As a result, it'd be better to leave the first static_cast as is. We can't say the same about the rest. They make no sense at all.

In the vector class, the [] operator takes an argument of the size_type type (typically an alias of size_t). The i variable value will automatically be converted to size_t, and there's nothing unsafe or suspicious about it. The explicit casts only add visual noise, so they're better removed.

for (std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1; i >= 0; --i) {
  strides[i] = acc;
  const auto sz = sizes[i];
  acc *= (sz == 0 ? 1 : sz);
}

You've taken the first step. But can it be simplified that long for the loop even further? Let's see—I'd like to write something like this:

for (auto sz : std::ranges::views::reverse(sizes)) {
  strides[???? i ????] = acc;
  acc *= (sz == 0 ? 1 : sz);
}

You still need the i variable to iterate through the strides array from the end, so we can't eliminate i completely. But we can do something like that:

int64_t acc = 1;
size_t i = nd;
for (auto sz : std::ranges::views::reverse(sizes)) {
  strides[--i] = acc;
  acc *= (sz == 0 ? 1 : sz);
}

To be honest, I don't like that --i has made the code more complicated and forces us to pause and realize why the variable is decremented in the beginning and then is used to access an array element.

Okay, there is less code now, and it's easier to scan through it quickly, so half the work is done. But the code is still ultimately hard to read—it's neither clearer nor intrinsically harder.

Let's take a look at the result.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  size_t i = nd;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--i] = acc;
    acc *= (sz == 0 ? 1 : sz);
  }

  int64_t ne = 1;

  for (auto s : sizes) {
    ne *= s;
    if (s == 0) {
      break;
    }
  }
  ....
}

Now that there is less code, it becomes clear that the second loop is redundant. In the first loop, we iterate through all the elements. So why not multiply them right away?

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  size_t i = nd;
  int64_t ne = 1;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--i] = acc;
    acc *= (sz == 0 ? 1 : sz);
    ne *= sz;
  }
  ....
}

Oh, it's an absolute beauty. It's not a big deal that we multiply all the elements together, even though one of them might be zero. Microprocessors are multiplying rapidly these days. You could lose more by re-accessing all the elements in the second loop.

What else is left? We don't need to initialize the strides container with zeros. All of its elements will be overwritten anyway.

std::vector<int64_t> strides(nd, 0); // delete the second argument

I think that's all. We can also make one more small change: remove the nd variable. It's not needed here or in the code that follows.

As a result, we have:

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  auto q = sizes.size();
  std::vector<int64_t> strides(q);
  int64_t acc = 1;
  int64_t ne = 1;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--q] = acc;
    acc *= (sz == 0 ? 1 : sz);
    ne *= sz;
  }
  ....
}

Hooray! We cut the code in half: from 20 lines down to just 9!

What's inside the assembly code?

The function is shorter now, but does that also improve performance? Yes! Let's take a look at the main section of the assembly code for the original function version.

Clang is used with the -O2 option:

.LBB1_7:
        mov     rax, rsi
        mov     qword ptr [rbx + 8*r12 - 16], rsi
        mov     rsi, qword ptr [r13 + 8*r12 - 16]
        cmp     rsi, 1
        adc     rsi, 0
        imul    rsi, rax
        dec     r12
        cmp     r12, 1
        ja      .LBB1_7
        mov     r12d, 1
        cmp     rbp, r13
        je      .LBB1_9
.LBB1_4:
        mov     rax, qword ptr [r13]
        test    rax, rax
        je      .LBB1_5
        imul    r12, rax
        add     r13, 8
        cmp     r13, rbp
        jne     .LBB1_4
        jmp     .LBB1_9

We can see a rather verbose assembly code with two loops. Now let's compare it with the assembly generated for the refactored version.

.LBB1_10:
        mov     rcx, rsi
        mov     rdx, qword ptr [rbp - 8]
        add     rbp, -8
        mov     qword ptr [rax], rsi
        cmp     rdx, 1
        mov     rsi, rdx
        adc     rsi, 0
        imul    rsi, rcx
        imul    r12, rdx
        add     rax, -8
        cmp     rbp, r13
        jne     .LBB1_10

Oh, that's pure beauty. One loop. Compact and efficient. Wow!

To those who aren't very familiar with the assembly language, it might seem strange that there isn't a conditional jump to implement this: sz == 0 ? 1 : sz.

Another way

Do we have another way? Yes, we have. I don't know whether I think about it or not. Unfortunately, after finishing my own refactoring, I made the mistake of immediately asking AI for alternative solutions.

DeepSeek suggestions weren't any better, and some were even worse than the original. For example, it optimized one of the code implementations by reversing the array.

// Wrap out strides
std::reverse(strides.begin(), strides.end());

But Claude Opus came up with an unusual implementation. It was noticed that we don't actually need two independent sequences of multiplications.

If all elements of the input array are non-zero, then at the end of the loop, acc == ne. And if even one element was zero, we can simply set ne to zero at the end. We return to the any_zero flag again, but in a smarter way. Using this idea, we can write the following code:

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  auto q = sizes.size();
  std::vector<int64_t> strides(q);
  int64_t acc = 1;
  bool any_zero = false;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--q] = acc;
    acc *= (sz == 0 ? 1 : sz);
    any_zero |= sz == 0;
  }
  const int64_t ne = any_zero ? 0 : acc;
  ....
}

Will code that performs half as many multiplications run faster? My prediction—not necessarily. The processor can perform two unrelated multiplications simultaneously on different pipelines. I'm willing to bet that my version and Claude's version will run at almost the same speed.

Performance measurement

Let's see how these implementations compare in reality. Designing a fair benchmark took a bit of effort. First, I had to ensure the input data couldn't overflow a 64-bit signed integer, since signed overflow is undefined behavior and would invalidate the measurements. Second, to make the contest fair, the test cases alternated between arrays with and without zero elements. Finally, the input arrays had varying lengths.

I'm not claiming that the measurements are rigorous, but they represent a general idea. The speed of the first original algorithm is taken as the baseline. I used Clang with the -O2 and -m64 options.

• The original version with two loops: 1.
• My version with a single loop: 0.88.
• The Claude version with a single multiplication: 0.92.

The refactoring improved performance by about 10% on average. I was actually hoping for a 20–30% speedup, but 10% is still a nice result—especially considering that it came from simplifying the code rather than making it more complicated.

Wrap-up

Let's stop our little refactoring adventure here. Was it worth it?

If we look at this from the side of this code enhancement, I think no. Tests are generated. It's non-critical that code is longer and slower than it could be.

It doesn't matter whether code is written by a human or generated by AI. One thing remains constant, someone must recognize what bad is and what good is. There must be experts who can spot the difference. Is the code good? Is it bad? Is the code fast? Is it secure?

Some people can accept just the fact that the code works—and who cares what's inside it. In some cases—for example, for building a prototype—this is reasonable. However, those who aren't interested in architecture, optimization, or security most likely simply don't see or understand the whole picture of developing and maintaining large software products. So, I think we'll likely see a lot of drawbacks on this topic.

AI is a powerful tool, but it's not a magic wand. When Claude suggested an alternative version of the algorithm to me, it was a good example of how AI can be used for research and to enhance human capabilities. But it can just as easily cause harm if used carelessly.

If you want to continue improving your skills as a C++ specialist and create reliable, secure software, I suggest you check out these useful e-books and webinars on our website:

E-books & Webinars

• C++ programmer's guide to undefined behavior.
• Terrible tips for a C++ developer.
• Webinar series: Let's make a programming language.