Skip to content

Wrong code at -O2/3 on x86_64-linux_gnu (a latent regression since Clang-15) #64047

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.

Sign up for GitHub

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

Jump to bottom
Closed
shao-hua-li opened this issue Jul 23, 2023 · 7 comments · Fixed by llvm/llvm-project-release-prs#676
Closed

Wrong code at -O2/3 on x86_64-linux_gnu (a latent regression since Clang-15) #64047

shao-hua-li opened this issue Jul 23, 2023 · 7 comments · Fixed by llvm/llvm-project-release-prs#676
Labels
miscompilation release:backport vectorizers
Milestone
LLVM 17.0.X Release

Comments

@shao-hua-li
Copy link

This looks like a long-standing issue since Clang-15. Clang at -O2/3 produces the wrong code. opt-bisect-limit suggests that the issue might be in LoopVectorizePass.

Compiler explorer: https://godbolt.org/z/dssMcdz55

% cat a.c
int printf(const char *, ...);
long a, c;
long *b = &a;
char d;
char *const e = &d;
int f, g, h;
int main() {
  long i[] = {1, 4, 4, 1, 4, 4};
  for (; g >= -6; g--) {
    f = 0;
    for (; f <= 5; f++)
      c = i[f] ^= *e > *b;
    *e = 1;
  }
  for (; h < 1; h++)
    printf("%d\n", (int)c);
}
%
% clang-tk -O0 a.c && ./a.out
4
% clang-tk -O2 a.c && ./a.out
1
% clang-tk -O3 a.c && ./a.out
1
%
% clang-tk -v
clang version 17.0.0 (https://github.com/llvm/llvm-project.git a5bba98a58b7406f81629d3942e03b1eff1e2b33)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /zdata/shaoli/compilers/ccbuilder-compilers
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/10
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
Candidate multilib: .;@m64
Selected multilib: .;@m64
%
@shao-hua-li
Copy link
Author

Bisect to 4e5e042, which was commited by @igogo-x86 .

@igogo-x86
Copy link
Contributor

@shao-hua-li, thank you! I'll look into it.

@igogo-x86
Copy link
Contributor

Pre-commit - https://reviews.llvm.org/D157630
Fix - https://reviews.llvm.org/D157631

@igogo-x86 igogo-x86 reopened this Aug 31, 2023
@igogo-x86 igogo-x86 added this to the LLVM 17.0.X Release milestone Aug 31, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Aug 31, 2023
@igogo-x86
Copy link
Contributor

/cherry-pick 2df9ed1 ac65fb8

@llvmbot
Copy link
Member

llvmbot commented Aug 31, 2023

/branch llvm/llvm-project-release-prs/issue64047

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 31, 2023
@igogo-x86 @llvmbot
[LoopVectorize] Fix incorrect order of invariant stores when there ar…
f03b6f9 
…e multiple reductions.

When a loop has multiple reductions, each with an intermediate invariant
store, the order in which those reductions are processed is not considered.
This can result in the invariant stores outside the loop not preserving the
original order.
This patch sorts VPReductionPHIRecipes by the order in which they have
stores in the original loop before running
`InnerLoopVectorizer::fixReduction` function, and it helps to maintain
the correct order of stores.

Fixes llvm/llvm-project#64047

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

(cherry picked from commit ac65fb869977185b44757b94dc5130bd08c6f7e2)
@llvmbot
Copy link
Member

llvmbot commented Aug 31, 2023

/branch llvm/llvm-project-release-prs/issue64047

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 31, 2023
@igogo-x86 @llvmbot
[LoopVectorize] Fix incorrect order of invariant stores when there ar…
94b98e1 
…e multiple reductions.

When a loop has multiple reductions, each with an intermediate invariant
store, the order in which those reductions are processed is not considered.
This can result in the invariant stores outside the loop not preserving the
original order.
This patch sorts VPReductionPHIRecipes by the order in which they have
stores in the original loop before running
`InnerLoopVectorizer::fixReduction` function, and it helps to maintain
the correct order of stores.

Fixes llvm/llvm-project#64047

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

(cherry picked from commit ac65fb869977185b44757b94dc5130bd08c6f7e2)
@llvmbot llvmbot mentioned this issue Aug 31, 2023
Merged
@llvmbot
Copy link
Member

llvmbot commented Aug 31, 2023

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

@tru tru moved this from Needs Triage to Needs Review in LLVM Release Status Sep 1, 2023
@tru tru moved this from Needs Review to Needs Merge in LLVM Release Status Sep 5, 2023
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Sep 11, 2023
@igogo-x86 @tru
[LoopVectorize] Fix incorrect order of invariant stores when there ar…
1174205 
…e multiple reductions.

When a loop has multiple reductions, each with an intermediate invariant
store, the order in which those reductions are processed is not considered.
This can result in the invariant stores outside the loop not preserving the
original order.
This patch sorts VPReductionPHIRecipes by the order in which they have
stores in the original loop before running
`InnerLoopVectorizer::fixReduction` function, and it helps to maintain
the correct order of stores.

Fixes llvm/llvm-project#64047

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

(cherry picked from commit ac65fb869977185b44757b94dc5130bd08c6f7e2)
@tru tru moved this from Needs Merge to Done in LLVM Release Status Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
miscompilation release:backport vectorizers
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 17.0.X Release
Development

Successfully merging a pull request may close this issue.

PR for llvm/llvm-project#64047 llvm/llvm-project-release-prs
4 participants
@shao-hua-li @EugeneZelenko @igogo-x86 @llvmbot