Would the compiler optimize this expression into a temporary constant rather than resolve it every iteration?

Question

I have the following loop:

for (unique_ptr<Surface>& child : Children)
{
    child->Gather(hit);

    if (hit && HitTestMode == HitTestMode::Content && child->MouseOver && !mouseOver)
    {
        mouseOver = true;
    }
}

I wonder if the compiler (I use Visual Studio 2013, targeting x64 on Win7 upwards) would optimize the expression

hit && HitTestMode == HitTestMode::Content

into a temporary constant and use that rather than resolve the expression every iteration, similar to me doing something like this:

bool contentMode = hit && HitTestMode == HitTestMode::Content;

for (unique_ptr<Surface>& child : Children)
{
    child->Gather(hit);

    if (contentMode && child->MouseOver && !mouseOver)
    {
        mouseOver = true;
    }
}

Bonus question:

Is checking for !mouseOver worth it (in order to skip the conditional mouseOver = true; if it has already been set)? Or is it faster to simply set it again regardless?

Probably not. Look into generated assembly code (with [GCC](http://gcc.gnu.org/) use `g++ -O -fverbose-asm -S`). Regarding performance, benchmark! (I think it is faster to simply set `mouseOver` again regardless) — Basile Starynkevitch, Sep 20 '14 at 23:58
answer to bonus question: it's faster to just set it again. I would add a break statement after assigning because that way you wouldn't have to go through the rest of the children. — programmerjake, Sep 21 '14 at 00:01
@programmerjake Thanks. I can't break, though, because there's an operation that needs to be performed on each child regardless (that I left out of my original example - I will update it to include this now). — d7samurai, Sep 21 '14 at 00:02
Here's a fun post on why if-statements can be expensive: http://stackoverflow.com/a/11227902/131930 — Jeremy Friesner, Sep 21 '14 at 00:17
@d7samurai What's a _"Bonus question"_ actually? Could you elaborate on this please? You're supposed to ask **one** question per question, and you can't give any bonuses for anyone on answers. Please stop that. You may rather ask another (related) question. Also for a better practice you shouldn't care about such things for micro-optimisation, unless you'll notice any real problems. Just let it go, and be productive instead. — πάντα ῥεῖ, Sep 21 '14 at 00:18
@d7samurai _"Would you be happier if it instead said "by the way..."?"_ Well, no. It's a different question from your original one. As mentioned: **one** question. — πάντα ῥεῖ, Sep 21 '14 at 00:27
@πάνταῥεῖ Well, it's a footnote, an apropos to the code, secondary, not warranting a separate question.. And as you can see, it has already been answered in a similarly secondary fashion - via a comment, rather than as an Answer. Calm down. — d7samurai, Sep 21 '14 at 00:29
I'm interested in the answer to this. You can compile in release mode with debug info and take a look at the disassembly. I'd try it but don't have time at the moment. — Neil Kirk, Sep 21 '14 at 00:45

score 1 · Accepted Answer · edited Sep 21 '14 at 17:43

The answer to whether that optimization could even take place would depend on what hit, HitTestMode and HitTestMode::Content are and whether it's possible that they could be changed by the call to child->Gather().

If those identifiers are constants or local variables that the compiler can prove aren't modified, then it's entirely possible that the sub-expression hit && HitTestMode == HitTestMode::Content will be hoisted.

For example, consider the following compilable version of your example:

#include <memory>
#include <vector>

using namespace std;

class Surface
{
public:
    void Gather(bool hit);
    bool MouseOver;
};

enum class HitTestMode
{
    Content = 1,
    Foo = 3,
    Bar = 4,
};


extern HitTestMode hittestmode;

bool anyMiceOver( vector<unique_ptr<Surface> > & Children, bool hit)
{
    bool mouseOver = false;

    for (unique_ptr<Surface>& child : Children)
    {
        child->Gather(hit);

        if (hit && hittestmode == HitTestMode::Content && child->MouseOver && !mouseOver)
        {
            mouseOver = true;
        }
    }

    return mouseOver;
}

When compiled using g++ 4.8.1 (mingw) with the -O3 optimization option, you get the following snippet of code for the loop (annotations added):

    mov rbx, QWORD PTR [rcx]      ; Children.begin()
    mov rsi, QWORD PTR 8[rcx]     ; Children.end()
    cmp rbx, rsi                 
    je  .L8                       ; early exit if Children is empty

    test    dl, dl                ; hit == 0?
    movzx   edi, dl
    je  .L5                       ; then goto loop L5

    xor ebp, ebp
    mov r12d, 1
    jmp .L7
    .p2align 4,,10
.L6:
    add rbx, 8
    cmp rsi, rbx                  ; check for end of Children
    je  .L2
.L7:
    mov rcx, QWORD PTR [rbx]
    mov edx, edi
    call    _ZN7Surface6GatherEb  ; call child->Gather(hit)
    cmp DWORD PTR hittestmode[rip], 1  ; check hittestmode
    jne .L6
    mov rax, QWORD PTR [rbx]      ; check child->MouseOver
    cmp BYTE PTR [rax], 0
    cmovne  ebp, r12d             ; set mouseOver appropriately
    jmp .L6
    .p2align 4,,10

.L5:                             ; loop L5 is run only when hit == 0
    mov rcx, QWORD PTR [rbx]     ; get net element in Children
    mov edx, edi
    add rbx, 8
    call    _ZN7Surface6GatherEb ; call child->Gather(hit)
    cmp rsi, rbx
    jne .L5

.L8:
    xor ebp, ebp
.L2:
    mov eax, ebp
    add rsp, 32
    pop rbx
    pop rsi
    pop rdi
    pop rbp
    pop r12
    ret

You'll note that the check for hit has been hoisted out of the loop - and if it's false then the a loop that does nothing but call child->Gather() is run.

If hitmodetest is changed to be a variable that's passed as a function argument so it's no longer subject to possibly being changed by the call to child-Gather(hit), then the compiler will also hoist the check for the value of hittestmode out of the loop and jump to the loop that does nothing but call child->Gather().

With a local hittestmode using -O2 will calculate hit && hittestmode == HitTestMode::Content prior to the loop and stash that result in a register, but it will still test the register in each loop iteration instead of optimizing to a separate loops that don't even bother with the test.

Since you specifically asked about the VS2013 compiler (using /Ox and /Ot options), it doesn't seem to hoist or optimize either of the checks (for hit or hittestmode) out of the loop - all it seems to do is keep the values for those variable in registers.

For our practical purposes here, your compilable example encapsulates exactly the way the actual implementation is structured, so your analysis is right on the money. Since I know something that the compiler doesn't (that `HitTestMode` isn't touched by `child->Gather()`) I can take the liberty of collapsing and moving them out of the loop. However, your answer has made me realize that I might _want_ `child->Gather()` to be able to [hackily] alter `HitTestMode` during iteration and therefore leave the expression in, "unhoisted".. Disappointing that VS doesn't hoist anything here, though. — d7samurai, Sep 21 '14 at 11:02

Would the compiler optimize this expression into a temporary constant rather than resolve it every iteration?

1 Answers1