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

Miscompilation with -opaque-pointers=true with LLVM16 #4538

Open
JohanEngelen opened this issue Nov 29, 2023 · 4 comments
Open

Miscompilation with -opaque-pointers=true with LLVM16 #4538

JohanEngelen opened this issue Nov 29, 2023 · 4 comments
Labels
llvm Related to LLVM

Comments

@JohanEngelen
Copy link
Member

Testcase from druntime rt/lifetime.d line 2686 miscompiles with -opaque-pointers=true -O:

static struct S
{
    S* thisptr;
    ~this() { assert(&this == thisptr); thisptr = null;}  // This assertion is triggered
}

void main() {
    S[] test14126 = new S[2048]; // make sure we allocate at least a PAGE
    foreach (ref s; test14126)
    {
        s.thisptr = &s;
    }
}

(no need to add this testcase when the bug is fixed, because it is already part of the testsuite when opaque pointers are enabled)

@JohanEngelen
Copy link
Member Author

This might be a bug in LLVM16.

Modified the testcase a little:

static struct S
{
    S* thisptr;
    ~this() {
        assert(&this == thisptr);
        thisptr = null;
    }
}

void foo() {
    S[] test14126 = new S[74];
    foreach (ref s; test14126)
    {
        s.thisptr = &s;
    }
    import core.stdc.stdio; printf("%p\n", &test14126[$-1]);
    import core.stdc.stdio; printf("%p\n", test14126[$-1].thisptr);
}

void main() {
    foo();
}

It bugs already in the printfs.

LLVM16 --opaque-pointers=true:
0x104224248
0x104224240 <-- mismatch!

LLVM17 gives correct output
0x10117c008
0x10117c008

I cannot find any fault in our outputted LLVM IR. Looks like a vectorization bug in LLVM16, because the optimized IR does look bad (writes same pointer to test14126[x].thisptr and test14126[x+1].thisptr).

@kinke
Copy link
Member

kinke commented Nov 30, 2023

OMG. And the CI-tested LLVM 16 version is our LDC-LLVM, based on v16.0.6, the latest patch for the v16 family. Ouch, so much for the plan to use LLVM 16 + opaque pointers by default in LDC v1.36. :(

@kinke
Copy link
Member

kinke commented Nov 30, 2023

FWIW, I've just pushed https://github.com/ldc-developers/llvm-project/tree/ldc-release/17.x (preliminary so far).

@JohanEngelen JohanEngelen changed the title Miscompilation with -opaque-pointers=true Miscompilation with -opaque-pointers=true with LLVM16 Dec 17, 2023
@JohanEngelen JohanEngelen added the llvm Related to LLVM label Dec 17, 2023
@kinke
Copy link
Member

kinke commented May 12, 2024

Also seen with LLVM v15.0.6 on Linux x86_64: https://github.com/ldc-developers/ldc/actions/runs/9052573385/job/24870398659?pr=4657

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm Related to LLVM
Projects
None yet
Development

No branches or pull requests

2 participants