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

[InstCombine] Miscompilation in InstCombinerImpl::foldSelectValueEquivalence #99436

Closed
dtcxzyw opened this issue Jul 18, 2024 · 0 comments · Fixed by #99492
Closed

[InstCombine] Miscompilation in InstCombinerImpl::foldSelectValueEquivalence #99436

dtcxzyw opened this issue Jul 18, 2024 · 0 comments · Fixed by #99492

Comments

@dtcxzyw
Copy link
Member

dtcxzyw commented Jul 18, 2024

Reproducer: https://godbolt.org/z/dzn8c8eYj

; bin/opt -passes=instcombine test.ll -S
; ModuleID = 'test.c'
source_filename = "test.c"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

@f = dso_local global [2 x i32] zeroinitializer, align 4
@__const.g.n = private unnamed_addr constant [1 x ptr] [ptr getelementptr (i8, ptr @f, i64 4)]

define void @g() {
entry:
  %n = alloca [1 x ptr], align 8
  call void @llvm.lifetime.start.p0(i64 8, ptr %n) #4
  call void @llvm.memcpy.p0.p0.i64(ptr align 8 %n, ptr align 8 @__const.g.n, i64 8, i1 false)
  %0 = load ptr, ptr %n, align 8
  %cmp3 = icmp eq ptr %0, null
  %1 = load i32, ptr %0, align 4
  %and = select i1 %cmp3, i32 %1, i32 0
  store i32 %and, ptr %0, align 4
  call void @llvm.lifetime.end.p0(i64 8, ptr %n) #4
  ret void
}
define void @g() {
  store i1 true, ptr poison, align 1
  ret void
}
IC: Visiting:   %cmp3 = icmp eq ptr getelementptr (i8, ptr @f, i64 4), null
IC: Visiting:   %0 = load i32, ptr getelementptr (i8, ptr @f, i64 4), align 4
IC: Visiting:   %and = select i1 %cmp3, i32 %0, i32 0
ADD DEFERRED:   %0 = load i32, ptr null, align 4
IC: Mod =   %and = select i1 %cmp3, i32 %0, i32 0
    New =   %and = select i1 %cmp3, i32 %0, i32 0
@dtcxzyw dtcxzyw self-assigned this Jul 18, 2024
@dtcxzyw dtcxzyw changed the title [InstCombine] Miscompilation in simplifyWithOpReplaced [InstCombine] Miscompilation in InstCombinerImpl::foldSelectValueEquivalence Jul 18, 2024
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this issue Jul 23, 2024
…m#99492)

Consider the following case:
```
%cmp = icmp eq ptr %p, null
%load = load i32, ptr %p, align 4
%sel = select i1 %cmp, i32 %load, i32 0
```
`foldSelectValueEquivalence` converts `load i32, ptr %p, align 4` into
`load i32, ptr null, align 4`, which causes immediate UB. `%load` is
speculatable, but it doesn't hold after operand substitution.

This patch introduces a new helper
`isSafeToSpeculativelyExecuteWithVariableReplaced`. It ignores operand
info in these instructions since their operands will be replaced later.

Fixes llvm#99436.

---------

Co-authored-by: Nikita Popov <[email protected]>
yuxuanchen1997 pushed a commit that referenced this issue Jul 25, 2024
Summary:
Consider the following case:
```
%cmp = icmp eq ptr %p, null
%load = load i32, ptr %p, align 4
%sel = select i1 %cmp, i32 %load, i32 0
```
`foldSelectValueEquivalence` converts `load i32, ptr %p, align 4` into
`load i32, ptr null, align 4`, which causes immediate UB. `%load` is
speculatable, but it doesn't hold after operand substitution.

This patch introduces a new helper
`isSafeToSpeculativelyExecuteWithVariableReplaced`. It ignores operand
info in these instructions since their operands will be replaced later.

Fixes #99436.

---------

Co-authored-by: Nikita Popov <[email protected]>

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251182
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant