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.

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

[LVI][CVP] CVP error deleted the and instruction. #68381

Closed
DianQK opened this issue Oct 6, 2023 · 3 comments · Fixed by #68190 or llvm/llvm-project-release-prs#728
Closed

[LVI][CVP] CVP error deleted the and instruction. #68381

DianQK opened this issue Oct 6, 2023 · 3 comments · Fixed by #68190 or llvm/llvm-project-release-prs#728
Assignees
@DianQK
Labels
llvm:optimizations miscompilation
Milestone
LLVM 17.0.X Release

Comments

@DianQK
Copy link
Member

DianQK commented Oct 6, 2023 •
edited
Loading

I tried this IR:

define i32 @foo(i1 %c0, i1 %c1, i8 %v1, i8 %v2) {
start:
  br i1 %c0, label %bb0, label %bb1

bb0:                                              ; preds = %bb1, %start
  %v2_i32 = zext i8 %v2 to i32
  br label %bb1

bb1:                                              ; preds = %bb0, %start
  %x = phi i32 [ %v2_i32, %bb0 ], [ undef, %start ]
  br i1 %c1, label %bb0, label %bb2

bb2:                                              ; preds = %bb1
  %v1_i32 = zext i8 %v1 to i32
  %y = or i32 %x, %v1_i32
  %z = and i32 %y, 255
  ret i32 %z
}

%z = and i32 %y, 255 should not be deleted by CVP.

alive2: https://alive2.llvm.org/ce/z/A9T1PH
godbolt: https://llvm.godbolt.org/z/YEqPjTv83

This issue started with LLVM 15.

This IR is reduced from https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Implementing.20niche.20checks.

Extended to abs: https://alive2.llvm.org/ce/z/k_8paG.
@DianQK DianQK self-assigned this Oct 6, 2023
@DianQK DianQK mentioned this issue Oct 4, 2023
Merged
@DianQK DianQK added this to the LLVM 17.0.X Release milestone Oct 6, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Oct 6, 2023
@tru tru moved this from Needs Triage to Needs Review in LLVM Release Status Oct 9, 2023
DianQK added a commit that referenced this issue Oct 10, 2023
@DianQK
[LVI][CVP] Treat undef like a full range (#68190)
8185794 
When converting to ConstantRange, we should treat undef like a full range.
Fixes #68381.
@DianQK DianQK reopened this Oct 10, 2023
@DianQK DianQK mentioned this issue Oct 10, 2023
Closed
@DianQK
Copy link
Member Author

DianQK commented Oct 10, 2023

While adding the abs test case, I found a new mis-compilation at #68683.
I need to fix this before adding abs test cases.

So let's do the backport first.

/cherry-pick 8185794

@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2023

/branch llvm/llvm-project-release-prs/issue68381

@llvmbot llvmbot mentioned this issue Oct 10, 2023
Merged
@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2023

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

@tru tru moved this from Needs Review to Done in LLVM Release Status Oct 16, 2023
tru pushed a commit that referenced this issue Oct 16, 2023
@DianQK @tru
[LVI][CVP] Treat undef like a full range (#68190)
d10b731 
When converting to ConstantRange, we should treat undef like a full range.
Fixes #68381.

(cherry picked from commit 8185794)
@saethlin saethlin mentioned this issue Oct 27, 2023
Closed
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 27, 2023
@bors
Auto merge of rust-lang#104862 - saethlin:mir-niche-checks, r=<try>
a926e3c 
Check for occupied niches

Implementation of rust-lang/compiler-team#624

Crater run has 62 crates that hit the check, 43 of those are published to crates.io. I see a lot of null function pointers and use of `mem::uninitialized` where the 0x1-filling collides with an enum niche.

But that is with full niche checks; checking transmute, plus any place where that we Copy, Move, or Inspect. Such checking is definitely too thorough to be on by default because it is 2x compile time overhead.

---

During implementation, this ran into llvm/llvm-project#68381

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 28, 2023
@bors
Auto merge of rust-lang#104862 - saethlin:mir-niche-checks, r=<try>
3ca7ba3 
Check for occupied niches

Implementation of rust-lang/compiler-team#624

Crater run has 62 crates that hit the check, 43 of those are published to crates.io. I see a lot of null function pointers and use of `mem::uninitialized` where the 0x1-filling collides with an enum niche.

But that is with full niche checks; checking transmute, plus any place where that we Copy, Move, or Inspect. Such checking is definitely too thorough to be on by default because it is 2x compile time overhead.

---

During implementation, this ran into llvm/llvm-project#68381

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 28, 2023
@bors
Auto merge of rust-lang#104862 - saethlin:mir-niche-checks, r=<try>
d53ed12 
Check for occupied niches

Implementation of rust-lang/compiler-team#624

Crater run has 62 crates that hit the check, 43 of those are published to crates.io. I see a lot of null function pointers and use of `mem::uninitialized` where the 0x1-filling collides with an enum niche.

But that is with full niche checks; checking transmute, plus any place where that we Copy, Move, or Inspect. Such checking is definitely too thorough to be on by default because it is 2x compile time overhead.

---

During implementation, this ran into llvm/llvm-project#68381

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 29, 2023
@bors
Auto merge of rust-lang#104862 - saethlin:mir-niche-checks, r=<try>
a3ce34e 
Check for occupied niches

Implementation of rust-lang/compiler-team#624

Crater run has 62 crates that hit the check, 43 of those are published to crates.io. I see a lot of null function pointers and use of `mem::uninitialized` where the 0x1-filling collides with an enum niche.

But that is with full niche checks; checking transmute, plus any place where that we Copy, Move, or Inspect. Such checking is definitely too thorough to be on by default because it is 2x compile time overhead.

---

During implementation, this ran into llvm/llvm-project#68381

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 29, 2023
@bors
Auto merge of rust-lang#104862 - saethlin:mir-niche-checks, r=<try>
57f535e 
Check for occupied niches

Implementation of rust-lang/compiler-team#624

Crater run has 62 crates that hit the check, 43 of those are published to crates.io. I see a lot of null function pointers and use of `mem::uninitialized` where the 0x1-filling collides with an enum niche.

But that is with full niche checks; checking transmute, plus any place where that we Copy, Move, or Inspect. Such checking is definitely too thorough to be on by default because it is 2x compile time overhead.

---

During implementation, this ran into llvm/llvm-project#68381

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 10, 2023
@bors
Auto merge of rust-lang#104862 - saethlin:mir-niche-checks, r=<try>
eaa4669 
Check for occupied niches

Implementation of rust-lang/compiler-team#624

Crater run has 62 crates that hit the check, 43 of those are published to crates.io. I see a lot of null function pointers and use of `mem::uninitialized` where the 0x1-filling collides with an enum niche.

But that is with full niche checks; checking transmute, plus any place where that we Copy, Move, or Inspect. Such checking is definitely too thorough to be on by default because it is 2x compile time overhead.

---

During implementation, this ran into llvm/llvm-project#68381

r? `@ghost`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@DianQK DianQK

Labels
llvm:optimizations miscompilation
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 17.0.X Release
2 participants
@DianQK @llvmbot