Skip to content

Commit e2c698c

Browse files
authored
[InstCombine] Fix miscompilation in sinkNotIntoLogicalOp (#142727)
Consider the following case: ``` define i1 @src(i8 %x) { %cmp = icmp slt i8 %x, -1 %not1 = xor i1 %cmp, true %or = or i1 %cmp, %not1 %not2 = xor i1 %or, true ret i1 %not2 } ``` `sinkNotIntoLogicalOp(%or)` calls `freelyInvert(%cmp, /*IgnoredUser=*/%or)` first. However, as `%cmp` is also used by `Op1 = %not1`, the RHS of `%or` is set to `%cmp.not = xor i1 %cmp, true`. Thus `Op1` is out of date in the second call to `freelyInvert`. Similarly, the second call may change `Op0`. According to the analysis above, I decided to avoid this fold when one of the operands is also a user of the other. Closes #142518.
1 parent 9ba332f commit e2c698c

File tree

2 files changed

+49
-0
lines changed

2 files changed

+49
-0
lines changed

llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4513,6 +4513,12 @@ bool InstCombinerImpl::sinkNotIntoLogicalOp(Instruction &I) {
45134513
if (Op0 == Op1)
45144514
return false;
45154515

4516+
// If one of the operands is a user of the other,
4517+
// freelyInvert->freelyInvertAllUsersOf will change the operands of I, which
4518+
// may cause miscompilation.
4519+
if (match(Op0, m_Not(m_Specific(Op1))) || match(Op1, m_Not(m_Specific(Op0))))
4520+
return false;
4521+
45164522
Instruction::BinaryOps NewOpc =
45174523
match(&I, m_LogicalAnd()) ? Instruction::Or : Instruction::And;
45184524
bool IsBinaryOp = isa<BinaryOperator>(I);
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -S -passes=instcombine < %s | FileCheck %s
3+
4+
define i8 @pr142518(ptr %p, i8 %x, i1 %c) "instcombine-no-verify-fixpoint" {
5+
; CHECK-LABEL: define i8 @pr142518(
6+
; CHECK-SAME: ptr [[P:%.*]], i8 [[X:%.*]], i1 [[C:%.*]]) #[[ATTR0:[0-9]+]] {
7+
; CHECK-NEXT: [[ENTRY:.*:]]
8+
; CHECK-NEXT: [[CMP1:%.*]] = icmp slt i8 [[X]], -1
9+
; CHECK-NEXT: br label %[[LOOP:.*]]
10+
; CHECK: [[LOOP]]:
11+
; CHECK-NEXT: [[NOT1:%.*]] = xor i1 [[CMP1]], true
12+
; CHECK-NEXT: [[OR:%.*]] = or i1 [[CMP1]], [[NOT1]]
13+
; CHECK-NEXT: [[CMP:%.*]] = xor i1 [[OR]], true
14+
; CHECK-NEXT: [[EXT2:%.*]] = zext i1 [[CMP]] to i8
15+
; CHECK-NEXT: store i8 [[EXT2]], ptr [[P]], align 1
16+
; CHECK-NEXT: br i1 false, label %[[LOOP]], label %[[EXIT:.*]]
17+
; CHECK: [[EXIT]]:
18+
; CHECK-NEXT: [[NOT3:%.*]] = xor i1 [[OR]], true
19+
; CHECK-NEXT: [[EXT3:%.*]] = zext i1 [[NOT3]] to i8
20+
; CHECK-NEXT: ret i8 [[EXT3]]
21+
;
22+
entry:
23+
%flag = alloca i8, align 1
24+
%cmp = icmp slt i8 %x, -1
25+
br label %loop
26+
27+
loop:
28+
%phi = phi i1 [ %cmp, %entry ], [ %c, %loop ]
29+
%not1 = xor i1 %phi, true
30+
%or = or i1 %cmp, %not1
31+
%not2 = xor i1 %or, true
32+
%ext2 = zext i1 %not2 to i8
33+
store i8 %ext2, ptr %p, align 1
34+
store i8 1, ptr %flag, align 1
35+
%flagv = load i8, ptr %flag, align 1
36+
%cond = icmp eq i8 %flagv, 0
37+
br i1 %cond, label %loop, label %exit
38+
39+
exit:
40+
%not3 = xor i1 %or, true
41+
%ext3 = zext i1 %not3 to i8
42+
ret i8 %ext3
43+
}

0 commit comments

Comments
 (0)