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

Incorrectly compiled vectorized code #64419

Closed
igogo-x86 opened this issue Aug 4, 2023 · 11 comments · Fixed by llvm/llvm-project-release-prs#558
Closed

Incorrectly compiled vectorized code #64419

igogo-x86 opened this issue Aug 4, 2023 · 11 comments · Fixed by llvm/llvm-project-release-prs#558
Assignees
Labels
backend:AArch64 bug Indicates an unexpected problem or unintended behavior miscompilation release:backport release:merged

Comments

@igogo-x86
Copy link
Contributor

The following code:

#include <stdio.h>

__attribute__((noinline)) void func(double *__restrict__ cme, double *__restrict__ qn, double *__restrict__ qsp, double *__restrict__ cwat, double *__restrict__ cmeres, double deltat, unsigned long long N) {
    for (unsigned long long i = 0; i < N; ++i) {
        if (qn[i] > 2.0)
            cme[i] = cme[i] + qn[i] * deltat;

       cme[i] = cme[i] + cmeres[i];
    }
}

int main() {
    double zero[] = {0.0, 0.0, 0.0, 0.0};
    double cme[] = {1.0, 1.0, 1.0, 1.0};
    double deltat = 1.0;
    double qn[] = {2.0, 2.0, 2.0, 2.0};
    double qsp[] = {2.0, 2.0, 2.0, 2.0};
    double cwat[] = {1.0, 1.0, 1.0, 1.0};

    func(cme, qn, /*qsp*/ qsp, /*cwat*/ zero, /* cmeres */zero, deltat, /*N*/4);
    
    printf("%f\n", cme[0]);
    return 0;
}

gives different results when vectorised and not-vectorised:

$ clang++ -mcpu=neoverse-v1 -march=armv8+sve -Ofast bug.c && ./a.out
2.000000
$ clang++ -mcpu=neoverse-v1 -march=armv8+sve -O0 bug.c && ./a.out
1.000000
@igogo-x86 igogo-x86 added bug Indicates an unexpected problem or unintended behavior llvm:codegen vectorization labels Aug 4, 2023
@igogo-x86 igogo-x86 added this to the LLVM 17.0.X Release milestone Aug 4, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 4, 2023

@llvm/issue-subscribers-bug

@igogo-x86 igogo-x86 self-assigned this Aug 4, 2023
@igogo-x86
Copy link
Contributor Author

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 4, 2023

@llvm/issue-subscribers-backend-aarch64

@tru
Copy link
Collaborator

tru commented Aug 9, 2023

@igogo-x86 should this be fixed in the release branch as well?

@igogo-x86
Copy link
Contributor Author

igogo-x86 commented Aug 9, 2023

@tru, yes, definitely. The final solution has two pre-commit with tests, and I hope this is the right way to cherry-pick it.

@igogo-x86 igogo-x86 reopened this Aug 9, 2023
@igogo-x86
Copy link
Contributor Author

/cherry-pick b560d5c
/cherry-pick 7542477
/cherry-pick 84d444f

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 9, 2023

Failed to cherry-pick: 84d444f

https://github.com/llvm/llvm-project/actions/runs/5806988783

Please manually backport the fix and push it to your github fork. Once this is done, please add a comment like this:

/branch <user>/<repo>/<branch>

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 9, 2023

/branch llvm/llvm-project-release-prs/issue64419

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 9, 2023

/pull-request llvm/llvm-project-release-prs#558

@nikic
Copy link
Contributor

nikic commented Aug 9, 2023

/cherry-pick b560d5c 7542477 84d444f

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 9, 2023

/branch llvm/llvm-project-release-prs/issue64419

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 9, 2023
* Remove the incorrect patterns from AArch64fmla_p/AArch64fmls_p
* Add correct patterns to AArch64fmla_m1/AArch64fmls_m1
* Refactor fma_patfrags for the sake of PatFrags

Fixes llvm/llvm-project#64419

Differential Revision: https://reviews.llvm.org/D157095

(cherry picked from commit 84d444f)
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 10, 2023
* Remove the incorrect patterns from AArch64fmla_p/AArch64fmls_p
* Add correct patterns to AArch64fmla_m1/AArch64fmls_m1
* Refactor fma_patfrags for the sake of PatFrags

Fixes llvm/llvm-project#64419

Differential Revision: https://reviews.llvm.org/D157095

(cherry picked from commit 84d444f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 bug Indicates an unexpected problem or unintended behavior miscompilation release:backport release:merged
Projects
Development

Successfully merging a pull request may close this issue.

6 participants