Skip to content

instcombine removes a select, making code more poisonous #91691

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
regehr opened this issue May 10, 2024 · 0 comments · Fixed by #91776
Closed

instcombine removes a select, making code more poisonous #91691

regehr opened this issue May 10, 2024 · 0 comments · Fixed by #91776
Assignees
@dtcxzyw
Labels
llvm:instcombine miscompilation

Comments

@regehr
Copy link
Contributor

regehr commented May 10, 2024 •
edited
Loading

https://alive2.llvm.org/ce/z/c__jy8

this function:

define i32 @f(i32 %0) {
  %2 = sub nuw i32 -2, %0
  %3 = tail call i32 @llvm.ctlz.i32(i32 %2, i1 false)
  %4 = sub i32 32, %3
  %5 = shl i32 1, %4
  %6 = icmp ult i32 %0, -2
  %7 = select i1 %6, i32 %5, i32 1
  ret i32 %7
}

; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
declare i32 @llvm.ctlz.i32(i32, i1 immarg) #0

attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

is getting rewritten to not have the select, but the select was blocking a poison value. the bad thing happens when -1 is passed as an argument, see the Alive link for a detailed execution trace

define i32 @f(i32 %0) {
  %2 = sub nuw i32 -2, %0
  %3 = tail call range(i32 0, 33) i32 @llvm.ctlz.i32(i32 %2, i1 false)
  %4 = sub nsw i32 0, %3
  %5 = and i32 %4, 31
  %6 = shl nuw i32 1, %5
  ret i32 %6
}

; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
declare i32 @llvm.ctlz.i32(i32, i1 immarg) #0

attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }

cc @nunoplopes
@dtcxzyw dtcxzyw self-assigned this May 10, 2024
@dtcxzyw dtcxzyw mentioned this issue May 10, 2024
Merged
dtcxzyw added a commit that referenced this issue May 13, 2024
@dtcxzyw
[InstCombine] Drop nuw flag when CtlzOp is a sub nuw (#91776)
b5f4210 
See the following case:
```
define i32 @SRC1(i32 %x) {
  %dec = sub nuw i32 -2, %x
  %ctlz = tail call i32 @llvm.ctlz.i32(i32 %dec, i1 false)
  %sub = sub nsw i32 32, %ctlz
  %shl = shl i32 1, %sub
  %ugt = icmp ult i32 %x, -2
  %sel = select i1 %ugt, i32 %shl, i32 1
  ret i32 %sel
}

define i32 @tgt1(i32 %x) {
  %dec = sub nuw i32 -2, %x
  %ctlz = tail call i32 @llvm.ctlz.i32(i32 %dec, i1 false)
  %sub = sub nsw i32 32, %ctlz
  %and = and i32 %sub, 31
  %shl = shl nuw i32 1, %and
  ret i32 %shl
}
```
`nuw` in `%dec` should be dropped after the select instruction is
eliminated.

Alive2: https://alive2.llvm.org/ce/z/7S9529

Fixes #91691.
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue May 13, 2024
@dtcxzyw @llvmbot
[InstCombine] Drop nuw flag when CtlzOp is a sub nuw (llvm#91776)
e4bbaa6 
See the following case:
```
define i32 @SRC1(i32 %x) {
  %dec = sub nuw i32 -2, %x
  %ctlz = tail call i32 @llvm.ctlz.i32(i32 %dec, i1 false)
  %sub = sub nsw i32 32, %ctlz
  %shl = shl i32 1, %sub
  %ugt = icmp ult i32 %x, -2
  %sel = select i1 %ugt, i32 %shl, i32 1
  ret i32 %sel
}

define i32 @tgt1(i32 %x) {
  %dec = sub nuw i32 -2, %x
  %ctlz = tail call i32 @llvm.ctlz.i32(i32 %dec, i1 false)
  %sub = sub nsw i32 32, %ctlz
  %and = and i32 %sub, 31
  %shl = shl nuw i32 1, %and
  ret i32 %shl
}
```
`nuw` in `%dec` should be dropped after the select instruction is
eliminated.

Alive2: https://alive2.llvm.org/ce/z/7S9529

Fixes llvm#91691.

(cherry picked from commit b5f4210)
tstellar pushed a commit to llvmbot/llvm-project that referenced this issue May 16, 2024
@dtcxzyw @tstellar
[InstCombine] Drop nuw flag when CtlzOp is a sub nuw (llvm#91776)
26ccc6b 
See the following case:
```
define i32 @SRC1(i32 %x) {
  %dec = sub nuw i32 -2, %x
  %ctlz = tail call i32 @llvm.ctlz.i32(i32 %dec, i1 false)
  %sub = sub nsw i32 32, %ctlz
  %shl = shl i32 1, %sub
  %ugt = icmp ult i32 %x, -2
  %sel = select i1 %ugt, i32 %shl, i32 1
  ret i32 %sel
}

define i32 @tgt1(i32 %x) {
  %dec = sub nuw i32 -2, %x
  %ctlz = tail call i32 @llvm.ctlz.i32(i32 %dec, i1 false)
  %sub = sub nsw i32 32, %ctlz
  %and = and i32 %sub, 31
  %shl = shl nuw i32 1, %and
  ret i32 %shl
}
```
`nuw` in `%dec` should be dropped after the select instruction is
eliminated.

Alive2: https://alive2.llvm.org/ce/z/7S9529

Fixes llvm#91691.

(cherry picked from commit b5f4210)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dtcxzyw dtcxzyw

Labels
llvm:instcombine miscompilation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[InstCombine] Drop nuw flag when CtlzOp is a sub nuw dtcxzyw/llvm-project
2 participants
@regehr @dtcxzyw