Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Issue 7387 - call instruction does not understand $ #11624

Merged
merged 1 commit into from Aug 27, 2020

Conversation

WalterBright
Copy link
Member

The matching order in the table was wrong, and the old 16 bit code generation was causing trouble.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
7387 normal call instruction does not understand $

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11624"

Comment on lines +1235 to +1245
//else if (popnd.disp >= short.min &&
//popnd.disp <= short.max && global.params.is16bit)
//us = CONSTRUCT_FLAGS(OpndSize._16, _rel, _flbl,0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remove ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to leave a reminder if I ever ported it back to DMC. That piece is necessary for 16 bit assembler.

@Geod24
Copy link
Member

Geod24 commented Aug 25, 2020

ubuntu/g++ doesn't like this: "block displacement of -157 exceeds the maximum offset of -128 to 127."

@WalterBright
Copy link
Member Author

Yeah, and of course there is no clue what file it was.

@WalterBright
Copy link
Member Author

I was hoping the problem would show up on one of the other tests.

@WalterBright
Copy link
Member Author

I'm ashamed to admit the "block displacement" message is coming from my code :-(

@MoonlightSentinel
Copy link
Contributor

Yeah, and of course there is no clue what file it was.

The failure happens when compiling std.internal.math.biguintx86 (which is explicitly listed in the log files).

A quick use of dustmite (+ manually copy+pasting a mixin) resulted in this reduced test case:

void multibyteMultiplyAccumulate() {

    static immutable int zero, storagenop ;

    asm {
        outer_loop:
        mov EBP, 0;
        mov ECX, 0;
        neg EBX;
        mov EAX, ESI;
        test EBX, 1;
        jnz L_enter_odd;
    }

    // mixin("asm pure nothrow @trusted { " ~ asmMulAdd_innerloop("add", "ESP") ~ "}");
    asm pure nothrow @trusted {
        // Entry point for even length
        add EBX, 1;
        mov EBP, ECX; // carry

        mul int ptr [ESP]; // M
        mov ECX, 0;

        add EBP, EAX;
        mov EAX, [ESI+4*EBX];
        adc ECX, EDX;

        mul int ptr [ESP]; // M
        add [-4+EDI+4*EBX], EBP;
        mov EBP, zero;

        adc ECX, EAX;
        mov EAX, [4+ESI+4*EBX];

        adc EBP, EDX;
        add EBX, 2;
        jnl L_done;
        L1:
        mul int ptr [ESP];
        add [-8+EDI+4*EBX], ECX;
        adc EBP, EAX;
        mov ECX, zero;
        mov EAX, [ESI+4*EBX];
        adc ECX, EDX;
        mov storagenop, EDX;
        mul int ptr [ESP];
        add [-4+EDI+4*EBX], EBP;
        mov EBP, zero;

        adc ECX, EAX;
        mov EAX, [4+ESI+4*EBX];

        adc EBP, EDX;
        add EBX, 2;
        jl L1;
        L_done: add [-8+EDI+4*EBX], ECX;
        adc EBP, 0;
    }

    asm {
        jmp outer_loop;
    }
    L_enter_odd:
}

@WalterBright
Copy link
Member Author

Thanks, @MoonlightSentinel . It looks like the jmp outer_loop is the problem.

@ghost
Copy link

ghost commented Aug 27, 2020

amend + force push please, in order to get it merged.

@WalterBright
Copy link
Member Author

autotester win32 failed with:

../dmd/generated/windows/release/32/dmd.exe -debug -debug=INVARIANT -debug=PTRCHECK -debug=PTRCHECK2 -m32 -g -conf= -Isrc -defaultlib=lib\druntime.lib -unittest -version=CoreUnittest -main -ofinvariant.exe src/gc/impl/conservative/gc.d src/rt/lifetime.d src/object.d
Error: linker exited with status 1

Gah, what does that have to do with this patch?

@ghost
Copy link

ghost commented Aug 27, 2020

what does that have to do with this patch?

Nothing but in this case the only way to get the PR merged is to restart the auto-test check.

@wilzbach
Copy link
Member

what does that have to do with this patch?

Nothing but in this case the only way to get the PR merged is to restart the auto-test check.

No, once auto-merge is applied the PR has a higher priority at the auto-tester and auto-tester will continuously build the PR until its merged. It's different to normal CIs.

@ghost ghost added the auto-merge label Aug 27, 2020
@dlang-bot dlang-bot merged commit ab1a350 into dlang:master Aug 27, 2020
@WalterBright WalterBright deleted the fix7387 branch August 27, 2020 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants