Redundant TEST instruction after ADD

[rameel]

godbolt | sharplab

Minimal repro:

public static void X(int value)
{
    while (true)
    {
        value -= 16;
        if (value <= 0) 
            break;
    }
}

coreclr trunk-20250720+5630010c13adad214fc369a365d0225d4b207ec6:

G_M42956_IG03:  ;; offset=0x0004
       add      edi, -16
       test     edi, edi ; <-- flags already set by ADD
       jg       SHORT G_M42956_IG03

Expected:

G_M42956_IG03:  ;; offset=0x0004
       add      edi, -16
       jg       SHORT G_M42956_IG03

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[xtqqczze]

   add      edi, -16
   jg       SHORT G_M42956_IG03

This sequence is not equivalent. The jg instruction depends on the condition (ZF = 0 && SF = OF). While add writes OF, test resets OF.

cc: @ashay

[xtqqczze]

   add      edi, -16
   test     edi, edi
   jg       SHORT G_M42956_IG03

I don’t think the test can be removed in this case. We also cannot rewrite the jump condition, since no single condition code directly captures the required flag combination.

cc: @EgorBo

[rameel]

This sequence is not equivalent. The jg instruction depends on the condition (ZF = 0 && SF = OF). While add writes OF, test resets OF.

I see now where I was mistaken. In my original example the input value was guaranteed to be positive, which made the extra test unnecessary. But when reducing it to a minimal repro, that guarantee was lost, so test is required to ensure correct behavior. Thanks for the clarification!

Just to confirm - is my understanding correct?

[xtqqczze]

We also cannot rewrite the jump condition

Clang doesn't find an optimisation here: https://cpp.godbolt.org/z/rMP1995bY