[llvm] e2c698c - [InstCombine] Fix miscompilation in `sinkNotIntoLogicalOp` (#142727)

[via llvm-commits]

Author: Yingwei Zheng
Date: 2025-06-04T17:48:01+08:00
New Revision: e2c698c7e836306f1a25c67597ae9e25a1fcc575

URL: https://github.com/llvm/llvm-project/commit/e2c698c7e836306f1a25c67597ae9e25a1fcc575
DIFF: https://github.com/llvm/llvm-project/commit/e2c698c7e836306f1a25c67597ae9e25a1fcc575.diff

LOG: [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 https://github.com/llvm/llvm-project/issues/142518.

Added: 
    llvm/test/Transforms/InstCombine/pr142518.ll

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 2fb4bfecda8aa..c6c231f81c4ab 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -4513,6 +4513,12 @@ bool InstCombinerImpl::sinkNotIntoLogicalOp(Instruction &I) {
   if (Op0 == Op1)
     return false;
 
+  // If one of the operands is a user of the other,
+  // freelyInvert->freelyInvertAllUsersOf will change the operands of I, which
+  // may cause miscompilation.
+  if (match(Op0, m_Not(m_Specific(Op1))) || match(Op1, m_Not(m_Specific(Op0))))
+    return false;
+
   Instruction::BinaryOps NewOpc =
       match(&I, m_LogicalAnd()) ? Instruction::Or : Instruction::And;
   bool IsBinaryOp = isa<BinaryOperator>(I);

diff  --git a/llvm/test/Transforms/InstCombine/pr142518.ll b/llvm/test/Transforms/InstCombine/pr142518.ll
new file mode 100644
index 0000000000000..980ed24b5eb98
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr142518.ll
@@ -0,0 +1,43 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+define i8 @pr142518(ptr %p, i8 %x, i1 %c) "instcombine-no-verify-fixpoint" {
+; CHECK-LABEL: define i8 @pr142518(
+; CHECK-SAME: ptr [[P:%.*]], i8 [[X:%.*]], i1 [[C:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp slt i8 [[X]], -1
+; CHECK-NEXT:    br label %[[LOOP:.*]]
+; CHECK:       [[LOOP]]:
+; CHECK-NEXT:    [[NOT1:%.*]] = xor i1 [[CMP1]], true
+; CHECK-NEXT:    [[OR:%.*]] = or i1 [[CMP1]], [[NOT1]]
+; CHECK-NEXT:    [[CMP:%.*]] = xor i1 [[OR]], true
+; CHECK-NEXT:    [[EXT2:%.*]] = zext i1 [[CMP]] to i8
+; CHECK-NEXT:    store i8 [[EXT2]], ptr [[P]], align 1
+; CHECK-NEXT:    br i1 false, label %[[LOOP]], label %[[EXIT:.*]]
+; CHECK:       [[EXIT]]:
+; CHECK-NEXT:    [[NOT3:%.*]] = xor i1 [[OR]], true
+; CHECK-NEXT:    [[EXT3:%.*]] = zext i1 [[NOT3]] to i8
+; CHECK-NEXT:    ret i8 [[EXT3]]
+;
+entry:
+  %flag = alloca i8, align 1
+  %cmp = icmp slt i8 %x, -1
+  br label %loop
+
+loop:
+  %phi = phi i1 [ %cmp, %entry ], [ %c, %loop ]
+  %not1 = xor i1 %phi, true
+  %or = or i1 %cmp, %not1
+  %not2 = xor i1 %or, true
+  %ext2 = zext i1 %not2 to i8
+  store i8 %ext2, ptr %p, align 1
+  store i8 1, ptr %flag, align 1
+  %flagv = load i8, ptr %flag, align 1
+  %cond = icmp eq i8 %flagv, 0
+  br i1 %cond, label %loop, label %exit
+
+exit:
+  %not3 = xor i1 %or, true
+  %ext3 = zext i1 %not3 to i8
+  ret i8 %ext3
+}