Issue 12500 - ICE in codegen when multiplying an incremented size_t by a double

[9rnsr]

https://issues.dlang.org/show_bug.cgi?id=12500

AST simplification will fix backend ICE, and additionally improve error diagnostic in issue 14102.

[yebblies]

Why is it valid for the lhs to be a CastExp? I didn't think this was a valid ast... Which expressions produce this, and is it a frontend bug that they get this far?

[9rnsr]

No, it's intended front-end result. Although it looks problematic as AST structure, but is necessary for the dmd backend codegen.
If the assigned lhs type is narrower than rhs, CastExp will be added. For example:

short a; int b;
a += b;  // Rewritten to: cast(int)a += b;
         // and it's equivalent with b = cast(short)(cast(int)a + b);

If we just remove the cast, backend ICE will happen, because a backend operand OPaddass cannot have short sized expression in its lhs.

I've tried to remove the lhs cast, but I couldn't do it.

[yebblies]

I don't see why it's necessary, the dmd backend limitation could (should) be worked around in the glue layer so other backends aren't burdened with supporting the invalid ast construct.

[9rnsr]

This is the workaround code for the dmd backend. ldc and gdc already have similar code.

https://github.com/D-Programming-GDC/GDC/blob/master/gcc/d/d-codegen.cc#L1738
https://github.com/ldc-developers/ldc/blob/master/gen/toir.cpp#L533

So, there is no need to fix it in a hurry.

[WalterBright]

Auto-merge toggled on

[9rnsr]

This PR introduced a regression: https://issues.dlang.org/show_bug.cgi?id=14220

However, I think it's caused by a hidden backend optimizer bug, and just unfortunately surfaced by this PR.

@WalterBright @yebblies I posted a reduced test case for 14220 in bugzilla. Can you see the backend issue?

[CyberShadow]

This pull request introduced a regression:
https://issues.dlang.org/show_bug.cgi?id=14341

[yebblies]

I don't think this is right, it will not evaluate the RHS at all.
eg

int ctfetest()
{
    int x;
    ++(x += 1);
    return x == 2;
}

void main()
{
    enum v = ctfetest();
    assert(v == ctfetest());
}

[9rnsr]

You're right. I filed the new CTFE issue: https://issues.dlang.org/show_bug.cgi?id=14371