fix return register of ?:

[WalterBright]

Unfortunately, github issues will not accept my keyboard input. Probably because I am using an older browser. Anyhow, Jared Hanson emailed me this bug report and fix, generated by AI and tweaked by myself:

---

• Context & In-place Mutation:

  In the x86 backend ( ), cdcond compiles ternary expressions (cond ? e21 : e22).

  codelem takes retregs by reference (ref regm_t pretregs), updating it in place to report which scratch register(s) the evaluated branch actually used.

  For branch 2 (e22), cdcond was designed to reuse retregs from branch 1 (e21) so that both branches leave their return value in the same register.

• Discard / Void Context (pretregs == 0):

  When a ternary expression is evaluated solely for side effects (discard context), its requested return registers are none (pretregs == 0).

  Branch 1 (e21) started with retregs = 0. While evaluating intermediate subexpressions, codelem mutated retregs from 0 to a scratch register (e.g., mAX).

  Because cdcond did not check if pretregs == 0 before evaluating branch 2, it passed retregs = mAX to branch 2 instead of 0.

• The 32-bit x86 Register Allocation Assertion:

  On 32-bit x86, dynamic arrays (string / darray) are 8 bytes ({ uint length, char* ptr }).

  8-byte values require a register pair consisting of a least-significant word register (mLSW, like EAX) and a most-significant word register (mMSW, like EDX).

  In testenumeration.d , branch 2 was msg ~ " world", returning an 8-byte string.

  Because codelem received retregs = mAX, the register allocator ( ) filtered retregs &= mMSW.

    Since mAX is not an mMSW register, retregs became 0, violating assert(retregs).
  

• Why the Fix Works

  Preserves Discard Intent:
  If the overall ternary expression is in discard context (pretregs == 0), resetting retregs = 0 guarantees branch 2 is also evaluated in discard context, ignoring any scratch registers populated by branch 1.

    Avoids Illegal Allocation Requests:
        With retregs = 0, codelem knows branch 2 does not need to store its final result into any specific target register.
        The register allocator never attempts to squeeze an 8-byte pair into a single 32-bit register (mAX), preventing the assert(retregs) failure and stopping UD2 emission.
  
    Leaves Valued Expressions Unchanged:
        When pretregs != 0 (the ternary result is actually consumed), the condition is skipped, preserving DMD's original behavior of aligning branch 2's return registers with branch 1.
  

[Herringway]

Would be nice to have a test for this.

[WalterBright]

Jared Hanson suggests the test case might be in this PR? #23744

[MetaLang]

It's tricky to give a definitive test case because this was in my experimental POC branch implementing switch expressions (#23744 as Walter said). The case that triggered this was:

switch (Value.Number(nextNumber()))
{
    case Number(number)   => number,
    case Wrapped(payload) => 0,
    case Empty()          => 0,
}

Which should desugar to:

struct Value
{
    ubyte __tag;

    union
    {
        struct __Payload_Number
        {
            int _0;
        }
        __Payload_Number __payload_0;

        struct __Payload_Wrapped
        {
            CopyCounted _0;
        }
        __Payload_Wrapped __payload_1;

        struct __Payload_Empty {}
        __Payload_Empty __payload_2;
    }

    static Value Number(int p0)
    {
        Value __result = void;
        __result.__payload_0 = __Payload_Number(p0);
        __result.__tag = 0;
        return __result;
    }

    static Value Wrapped(CopyCounted p0)
    {
        Value __result = void;
        __result.__payload_1 = __Payload_Wrapped(p0);
        __result.__tag = 1;
        return __result;
    }

    @property static Value Empty()
    {
        Value __result = void;
        __result.__tag = 2;
        return __result;
    }

    this(ref typeof(this) rhs)
    {
        this.__tag = rhs.__tag;
        switch (rhs.__tag)
        {
            case 0:
                this.__payload_0 = rhs.__payload_0;
                break;
            case 1:
                this.__payload_1 = __Payload_Wrapped(rhs.__payload_1._0);
                break;
            case 2:
            default:
                break;
        }
    }

    ~this()
    {
        switch (this.__tag)
        {
            case 1:
                destroy(this.__payload_1._0);
                break;
            default:
                break;
        }
    }
}

(
    Value __switch = Value.Number(nextNumber()),
    (__switch.__tag == 0)
        ? (
            int number = __switch.__payload_0._0,
            number
          )
        : (__switch.__tag == 1)
            ? (
                CopyCounted payload = __switch.__payload_1._0,
                0
              )
            : (
                0
              )
);

This should be a minimal test case, but I can't guarantee it reproduces the issue:

int sideEffect() { return 42; }

void main()
{
    int cond1 = 1, cond2 = 0;
    int var1 = 10, var2 = 20;

    // Discarded value (pretregs == 0 in the backend)
    cast(void)(cond1
        ? var1                          // 1. Evaluated into EAX (retregs becomes mAX)
        : (cond2 ? sideEffect() : var2) // 2. Nested ternary incorrectly inherits retregs = mAX
    );
}

[WalterBright]

the minimal test case does not cause an assert fail

[MetaLang]

Okay I'll see if I can reproduce it again in my own branch and reduce that down to a minimal test case.

[MetaLang]

Here is a minimal example that crashes with current DMD (I was able to reproduce this on run.dlang.io):

void test(bool cond, string msg)
{
    string text = cond ? "empty" : (msg ~ " world");
}

void main()
{
    test(false, "hello");
}

It requires the compiler flags -m32 -O

[thewilsonator]

Please open this as an issue so that this PR can close it.

[MetaLang]

#23976