[InstCombine] Fix buggy transform in foldNestedSelects; PR 71330 #71489

goldsteinn · 2023-11-07T05:45:01Z

[InstCombine] Add reduced test for PR 71330; NFC
[InstCombine] Fix buggy transform in foldNestedSelects; PR 71330

llvmbot · 2023-11-07T05:45:32Z

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

[InstCombine] Add reduced test for PR 71330; NFC
[InstCombine] Fix buggy transform in foldNestedSelects; PR 71330

Full diff: https://github.com/llvm/llvm-project/pull/71489.diff

2 Files Affected:

(modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+9-2)
(added) llvm/test/Transforms/InstCombine/pr71330.ll (+229)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 9bd49f76d4bd5b7..71c2d68881441ac 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -2891,8 +2891,15 @@ static Instruction *foldNestedSelects(SelectInst &OuterSelVal,
     std::swap(InnerSel.TrueVal, InnerSel.FalseVal);
 
   Value *AltCond = nullptr;
-  auto matchOuterCond = [OuterSel, &AltCond](auto m_InnerCond) {
-    return match(OuterSel.Cond, m_c_LogicalOp(m_InnerCond, m_Value(AltCond)));
+  auto matchOuterCond = [OuterSel, IsAndVariant, &AltCond](auto m_InnerCond) {
+    // An unsimplified select condition can match both LogicalAnd and LogicalOr
+    // (select true, true, false). Since below we assume that LogicalAnd implies
+    // InnerSel match the FVal and vice versa for LogicalOr, we can't match the
+    // alternative pattern here.
+    return IsAndVariant ? match(OuterSel.Cond,
+                                m_c_LogicalAnd(m_InnerCond, m_Value(AltCond)))
+                        : match(OuterSel.Cond,
+                                m_c_LogicalOr(m_InnerCond, m_Value(AltCond)));
   };
 
   // Finally, match the condition that was driving the outermost `select`,
diff --git a/llvm/test/Transforms/InstCombine/pr71330.ll b/llvm/test/Transforms/InstCombine/pr71330.ll
new file mode 100644
index 000000000000000..635eb42182ec642
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr71330.ll
@@ -0,0 +1,229 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
+; RUN: opt -O3 -S < %s | FileCheck %s
+
+@e = global i8 5
+@c = global i64 0
+@k = global i8 0
+@n = global i16 0
+@o = global i32 0
+
+define void @pr71330(ptr %s, i32 %.pre, i8 %0) {
+; CHECK-LABEL: define void @pr71330(
+; CHECK-SAME: ptr nocapture writeonly [[S:%.*]], i32 [[DOTPRE:%.*]], i8 [[TMP0:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[DOTPRE]], 0
+; CHECK-NEXT:    br i1 [[TOBOOL_NOT]], label [[FOR_COND5_PREHEADER:%.*]], label [[COMMON_RET:%.*]]
+; CHECK:       for.cond5.preheader:
+; CHECK-NEXT:    [[TOBOOL28_NOT:%.*]] = icmp eq i8 [[TMP0]], 0
+; CHECK-NEXT:    [[TMP1:%.*]] = load i8, ptr @e, align 1
+; CHECK-NEXT:    [[DOTFR:%.*]] = freeze i8 [[TMP1]]
+; CHECK-NEXT:    [[CONV:%.*]] = sext i8 [[DOTFR]] to i32
+; CHECK-NEXT:    [[C_PROMOTED:%.*]] = load i64, ptr @c, align 8
+; CHECK-NEXT:    [[K_PROMOTED:%.*]] = load i8, ptr @k, align 1
+; CHECK-NEXT:    [[TOBOOL19_NOT1:%.*]] = icmp eq i8 [[DOTFR]], 0
+; CHECK-NEXT:    [[ADD9_US:%.*]] = add nsw i32 [[CONV]], 9
+; CHECK-NEXT:    br i1 [[TOBOOL19_NOT1]], label [[FOR_COND7_PREHEADER_SPLIT_US_SPLIT:%.*]], label [[FOR_COND7_PREHEADER10:%.*]]
+; CHECK:       common.ret:
+; CHECK-NEXT:    ret void
+; CHECK:       for.cond7.preheader10:
+; CHECK-NEXT:    [[CONV10:%.*]] = sext i32 [[ADD9_US]] to i64
+; CHECK-NEXT:    [[INC:%.*]] = or i8 [[K_PROMOTED]], 1
+; CHECK-NEXT:    [[INC24:%.*]] = or i64 [[C_PROMOTED]], 1
+; CHECK-NEXT:    [[ADD1112:%.*]] = or i64 [[C_PROMOTED]], [[CONV10]]
+; CHECK-NEXT:    [[CMP1213:%.*]] = icmp slt i64 [[ADD1112]], 15
+; CHECK-NEXT:    br i1 [[CMP1213]], label [[FOR_BODY14_PREHEADER:%.*]], label [[FOR_COND27_PREHEADER:%.*]]
+; CHECK:       for.body14.preheader:
+; CHECK-NEXT:    store i8 [[INC]], ptr @k, align 1
+; CHECK-NEXT:    store i16 0, ptr @n, align 2
+; CHECK-NEXT:    store i32 0, ptr @o, align 4
+; CHECK-NEXT:    store i64 [[INC24]], ptr @c, align 8
+; CHECK-NEXT:    [[ADD11:%.*]] = or i64 [[INC24]], [[CONV10]]
+; CHECK-NEXT:    [[CMP12:%.*]] = icmp slt i64 [[ADD11]], 15
+; CHECK-NEXT:    br label [[FOR_BODY14:%.*]]
+; CHECK:       for.cond7.preheader.split.us.split:
+; CHECK-NEXT:    [[CONV10_US:%.*]] = zext nneg i32 [[ADD9_US]] to i64
+; CHECK-NEXT:    [[ADD11_US4:%.*]] = or i64 [[C_PROMOTED]], [[CONV10_US]]
+; CHECK-NEXT:    [[CMP12_US5:%.*]] = icmp slt i64 [[ADD11_US4]], 15
+; CHECK-NEXT:    br i1 [[CMP12_US5]], label [[FOR_BODY14_US_LR_PH:%.*]], label [[FOR_COND27_PREHEADER]]
+; CHECK:       for.body14.us.lr.ph:
+; CHECK-NEXT:    store i16 0, ptr @n, align 2
+; CHECK-NEXT:    [[INC24_US:%.*]] = or i64 [[C_PROMOTED]], 1
+; CHECK-NEXT:    [[ADD11_US:%.*]] = or i64 [[INC24_US]], [[CONV10_US]]
+; CHECK-NEXT:    [[CMP12_US:%.*]] = icmp slt i64 [[ADD11_US]], 15
+; CHECK-NEXT:    br label [[FOR_BODY14_US:%.*]]
+; CHECK:       for.body14.us:
+; CHECK-NEXT:    br i1 [[CMP12_US]], label [[FOR_BODY14_US]], label [[FOR_COND7_US_FOR_COND27_PREHEADER_SPLIT_US_SPLIT_CRIT_EDGE:%.*]]
+; CHECK:       for.cond7.us.for.cond27.preheader.split.us.split_crit_edge:
+; CHECK-NEXT:    [[INC_US:%.*]] = or i8 [[K_PROMOTED]], 1
+; CHECK-NEXT:    store i8 [[INC_US]], ptr @k, align 1
+; CHECK-NEXT:    store i64 [[INC24_US]], ptr @c, align 8
+; CHECK-NEXT:    br label [[FOR_COND27_PREHEADER]]
+; CHECK:       for.cond27.preheader:
+; CHECK-NEXT:    br i1 [[TOBOOL28_NOT]], label [[COMMON_RET]], label [[FOR_INC30_LR_PH:%.*]]
+; CHECK:       for.inc30.lr.ph:
+; CHECK-NEXT:    store i8 0, ptr [[S]], align 1
+; CHECK-NEXT:    br label [[FOR_INC30:%.*]]
+; CHECK:       for.body14:
+; CHECK-NEXT:    br label [[FOR_BODY20:%.*]]
+; CHECK:       for.body20:
+; CHECK-NEXT:    [[TMP2:%.*]] = phi i16 [ 0, [[FOR_BODY14]] ], [ [[INC21:%.*]], [[FOR_BODY20]] ]
+; CHECK-NEXT:    [[INC21]] = add i16 [[TMP2]], 1
+; CHECK-NEXT:    [[CONV17:%.*]] = sext i16 [[INC21]] to i32
+; CHECK-NEXT:    [[ADD18:%.*]] = sub nsw i32 0, [[CONV17]]
+; CHECK-NEXT:    [[TOBOOL19_NOT:%.*]] = icmp eq i32 [[CONV]], [[ADD18]]
+; CHECK-NEXT:    br i1 [[TOBOOL19_NOT]], label [[FOR_COND15_FOR_INC23_CRIT_EDGE:%.*]], label [[FOR_BODY20]]
+; CHECK:       for.cond15.for.inc23_crit_edge:
+; CHECK-NEXT:    br i1 [[CMP12]], label [[FOR_BODY14]], label [[FOR_COND27_PREHEADER]]
+; CHECK:       for.inc30:
+; CHECK-NEXT:    br label [[FOR_INC30]]
+;
+entry:
+  %tobool.not = icmp eq i32 %.pre, 0
+  br i1 %tobool.not, label %for.cond5, label %common.ret
+
+common.ret:  ; preds = %for.cond5, %entry
+  ret void
+
+for.cond5:  ; preds = %for.cond27, %entry
+  %storemerge4 = phi i32 [ -3, %entry ], [ %.pre, %for.cond27 ]
+  %cmp = icmp slt i32 %storemerge4, 0
+  br i1 %cmp, label %for.cond7, label %common.ret
+
+for.cond7:  ; preds = %for.inc23, %for.cond5
+  %add = add nsw i32 %storemerge4, 8
+  %and.i.i = and i32 %add, 8
+  %cmp.i.i = icmp eq i32 %and.i.i, 0
+  br i1 %cmp.i.i, label %fn3.exit, label %if.end.i.i
+
+if.end.i.i:  ; preds = %for.cond7
+  %and2.i.i = and i32 %add, 12
+  %cmp3.i.i = icmp eq i32 %and2.i.i, 0
+  br i1 %cmp3.i.i, label %fn3.exit, label %if.end5.i.i
+
+if.end5.i.i:  ; preds = %if.end.i.i
+  %and7.i.i = and i32 %storemerge4, 1
+  %cmp8.i.i = icmp eq i32 %and7.i.i, 0
+  br i1 %cmp8.i.i, label %fn3.exit, label %if.end10.i.i
+
+if.end10.i.i:  ; preds = %if.end5.i.i
+  %tobool.not.i.i = icmp eq i32 %add, 0
+  %..i.i = select i1 %tobool.not.i.i, i32 0, i32 3
+  br label %fn3.exit
+
+fn3.exit:  ; preds = %if.end10.i.i, %if.end5.i.i, %if.end.i.i, %for.cond7
+  %retval.0.i.i = phi i32 [ 0, %for.cond7 ], [ 1, %if.end.i.i ], [ 0, %if.end5.i.i ], [ %..i.i, %if.end10.i.i ]
+  %1 = load i8, ptr @e, align 1
+  %conv = sext i8 %1 to i32
+  %add8 = or i32 %conv, %retval.0.i.i
+  %add9 = add i32 %add8, 9
+  %conv10 = sext i32 %add9 to i64
+  %2 = load i64, ptr @c, align 8
+  %add11 = or i64 %conv10, %2
+  %cmp12 = icmp slt i64 %add11, 15
+  br i1 %cmp12, label %for.body14, label %for.cond27
+
+for.body14:  ; preds = %fn3.exit
+  %3 = load i8, ptr @k, align 1
+  %inc = or i8 %3, 1
+  store i8 %inc, ptr @k, align 1
+  br label %for.cond15
+
+for.cond15:  ; preds = %for.body20, %for.body14
+  %4 = phi i16 [ 0, %for.body14 ], [ %inc21, %for.body20 ]
+  store i16 0, ptr @n, align 2
+  %conv17 = sext i16 %4 to i32
+  %add18 = sub i32 0, %conv17
+  %tobool19.not = icmp eq i32 %conv, %add18
+  br i1 %tobool19.not, label %for.inc23, label %for.body20
+
+for.body20:  ; preds = %for.cond15
+  store i32 0, ptr @o, align 4
+  %inc21 = add i16 %4, 1
+  br label %for.cond15
+
+for.inc23:  ; preds = %for.cond15
+  %inc24 = or i64 %2, 1
+  store i64 %inc24, ptr @c, align 8
+  br label %for.cond7
+
+for.cond27:  ; preds = %for.inc30, %fn3.exit
+  %tobool28.not = icmp eq i8 %0, 0
+  br i1 %tobool28.not, label %for.cond5, label %for.inc30
+
+for.inc30:  ; preds = %for.cond27
+  store i8 0, ptr %s, align 1
+  br label %for.cond27
+}
+
+define i32 @pr71330_main() local_unnamed_addr {
+; CHECK-LABEL: define i32 @pr71330_main(
+; CHECK-SAME: ) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i8, ptr @e, align 1
+; CHECK-NEXT:    [[DOTFR_I:%.*]] = freeze i8 [[TMP0]]
+; CHECK-NEXT:    [[CONV_I:%.*]] = sext i8 [[DOTFR_I]] to i32
+; CHECK-NEXT:    [[C_PROMOTED_I:%.*]] = load i64, ptr @c, align 8
+; CHECK-NEXT:    [[K_PROMOTED_I:%.*]] = load i8, ptr @k, align 1
+; CHECK-NEXT:    [[TOBOOL19_NOT1_I:%.*]] = icmp eq i8 [[DOTFR_I]], 0
+; CHECK-NEXT:    [[ADD9_US_I:%.*]] = add nsw i32 [[CONV_I]], 9
+; CHECK-NEXT:    br i1 [[TOBOOL19_NOT1_I]], label [[FOR_COND7_PREHEADER_SPLIT_US_SPLIT_I:%.*]], label [[FOR_COND7_PREHEADER10_I:%.*]]
+; CHECK:       for.cond7.preheader10.i:
+; CHECK-NEXT:    [[CONV10_I:%.*]] = sext i32 [[ADD9_US_I]] to i64
+; CHECK-NEXT:    [[ADD11_I1:%.*]] = or i64 [[C_PROMOTED_I]], [[CONV10_I]]
+; CHECK-NEXT:    [[CMP12_I2:%.*]] = icmp slt i64 [[ADD11_I1]], 15
+; CHECK-NEXT:    br i1 [[CMP12_I2]], label [[FOR_BODY14_I_LR_PH:%.*]], label [[PR71330_EXIT:%.*]]
+; CHECK:       for.body14.i.lr.ph:
+; CHECK-NEXT:    [[INC24_I:%.*]] = or i64 [[C_PROMOTED_I]], 1
+; CHECK-NEXT:    [[INC_I:%.*]] = or i8 [[K_PROMOTED_I]], 1
+; CHECK-NEXT:    store i8 [[INC_I]], ptr @k, align 1
+; CHECK-NEXT:    store i16 0, ptr @n, align 2
+; CHECK-NEXT:    store i32 0, ptr @o, align 4
+; CHECK-NEXT:    store i64 [[INC24_I]], ptr @c, align 8
+; CHECK-NEXT:    [[ADD11_I:%.*]] = or i64 [[INC24_I]], [[CONV10_I]]
+; CHECK-NEXT:    [[CMP12_I:%.*]] = icmp slt i64 [[ADD11_I]], 15
+; CHECK-NEXT:    br i1 [[CMP12_I]], label [[FOR_BODY14_I_US:%.*]], label [[FOR_BODY20_I:%.*]]
+; CHECK:       for.body14.i.us:
+; CHECK-NEXT:    br label [[FOR_BODY20_I_US:%.*]]
+; CHECK:       for.body20.i.us:
+; CHECK-NEXT:    [[TMP1:%.*]] = phi i16 [ 0, [[FOR_BODY14_I_US]] ], [ [[INC21_I_US:%.*]], [[FOR_BODY20_I_US]] ]
+; CHECK-NEXT:    [[INC21_I_US]] = add i16 [[TMP1]], 1
+; CHECK-NEXT:    [[CONV17_I_US:%.*]] = sext i16 [[INC21_I_US]] to i32
+; CHECK-NEXT:    [[ADD18_I_US:%.*]] = sub nsw i32 0, [[CONV17_I_US]]
+; CHECK-NEXT:    [[TOBOOL19_NOT_I_US:%.*]] = icmp eq i32 [[CONV_I]], [[ADD18_I_US]]
+; CHECK-NEXT:    br i1 [[TOBOOL19_NOT_I_US]], label [[FOR_BODY14_I_US]], label [[FOR_BODY20_I_US]]
+; CHECK:       for.cond7.preheader.split.us.split.i:
+; CHECK-NEXT:    [[CONV10_US_I:%.*]] = zext nneg i32 [[ADD9_US_I]] to i64
+; CHECK-NEXT:    [[ADD11_US4_I:%.*]] = or i64 [[C_PROMOTED_I]], [[CONV10_US_I]]
+; CHECK-NEXT:    [[CMP12_US5_I:%.*]] = icmp slt i64 [[ADD11_US4_I]], 15
+; CHECK-NEXT:    br i1 [[CMP12_US5_I]], label [[FOR_BODY14_US_LR_PH_I:%.*]], label [[PR71330_EXIT]]
+; CHECK:       for.body14.us.lr.ph.i:
+; CHECK-NEXT:    store i16 0, ptr @n, align 2
+; CHECK-NEXT:    [[INC24_US_I:%.*]] = or i64 [[C_PROMOTED_I]], 1
+; CHECK-NEXT:    [[ADD11_US_I:%.*]] = or i64 [[INC24_US_I]], [[CONV10_US_I]]
+; CHECK-NEXT:    [[CMP12_US_I:%.*]] = icmp slt i64 [[ADD11_US_I]], 15
+; CHECK-NEXT:    br i1 [[CMP12_US_I]], label [[FOR_BODY14_US_I:%.*]], label [[FOR_COND7_US_FOR_COND27_PREHEADER_SPLIT_US_SPLIT_CRIT_EDGE_I:%.*]]
+; CHECK:       for.body14.us.i:
+; CHECK-NEXT:    br label [[FOR_BODY14_US_I]]
+; CHECK:       for.cond7.us.for.cond27.preheader.split.us.split_crit_edge.i:
+; CHECK-NEXT:    [[INC_US_I:%.*]] = or i8 [[K_PROMOTED_I]], 1
+; CHECK-NEXT:    store i8 [[INC_US_I]], ptr @k, align 1
+; CHECK-NEXT:    store i64 [[INC24_US_I]], ptr @c, align 8
+; CHECK-NEXT:    br label [[PR71330_EXIT]]
+; CHECK:       for.body20.i:
+; CHECK-NEXT:    [[TMP2:%.*]] = phi i16 [ [[INC21_I:%.*]], [[FOR_BODY20_I]] ], [ 0, [[FOR_BODY14_I_LR_PH]] ]
+; CHECK-NEXT:    [[INC21_I]] = add i16 [[TMP2]], 1
+; CHECK-NEXT:    [[CONV17_I:%.*]] = sext i16 [[INC21_I]] to i32
+; CHECK-NEXT:    [[ADD18_I:%.*]] = sub nsw i32 0, [[CONV17_I]]
+; CHECK-NEXT:    [[TOBOOL19_NOT_I:%.*]] = icmp eq i32 [[CONV_I]], [[ADD18_I]]
+; CHECK-NEXT:    br i1 [[TOBOOL19_NOT_I]], label [[PR71330_EXIT]], label [[FOR_BODY20_I]]
+; CHECK:       pr71330.exit:
+; CHECK-NEXT:    [[TMP3:%.*]] = phi i8 [ [[K_PROMOTED_I]], [[FOR_COND7_PREHEADER10_I]] ], [ [[K_PROMOTED_I]], [[FOR_COND7_PREHEADER_SPLIT_US_SPLIT_I]] ], [ [[INC_US_I]], [[FOR_COND7_US_FOR_COND27_PREHEADER_SPLIT_US_SPLIT_CRIT_EDGE_I]] ], [ [[INC_I]], [[FOR_BODY20_I]] ]
+; CHECK-NEXT:    [[CONV1:%.*]] = sext i8 [[TMP3]] to i32
+; CHECK-NEXT:    ret i32 [[CONV1]]
+;
+entry:
+  call void @pr71330(ptr null, i32 0, i8 0)
+  %0 = load i8, ptr @k, align 1
+  %conv1 = sext i8 %0 to i32
+  ret i32 %conv1
+}

llvm/test/Transforms/InstCombine/pr71330.ll

goldsteinn · 2023-11-07T19:47:57Z

llvm/test/Transforms/InstCombine/pr71330.ll

+
+; uselistorder directives
+  uselistorder i32 %storemerge33, { 2, 4, 1, 3, 0 }
+}


I ended up reducing by hand, llvm-reduce simply doesn't change anything.

What kind of interestingness test were you using? Diff between two opt binaries?

I put an assertion in at the point of the bug and any change that triggered the assert.

#! /usr/bin/env python3 import os import sys from common_util import run_proc def os_do(cmd): print(cmd) return os.system(cmd) s, r, stdout, stderr = run_proc( "./bin/opt -passes=instcombine {} -o unused.ll".format(sys.argv[1]), timeout=10, decode=True) print("R={}".format(r)) if s is False: sys.exit(1) if r != 134: sys.exit(1) print(stderr) if "UID:1234567890" in stderr: sys.exit(0) sys.exit(1)

Edit: the UID:.... is message I put on the assert.

Hm yeah, for some reason llvm-reduce is completely ineffective for this case. What worked for me is to manually remove uses of storemerge33 and then reorder them so the uselistorder is no longer necessary, and then run llvm-reduce afterwards. This gave me: https://gist.github.com/nikic/da8251d05a5df6038bd3e2eb773f64c0 This is probably still nowhere near minimal, but maybe good enough for a test case.

Updated to your test case.

@bad

The bug is that `IsAndVariant` is used to assume which arm in the select the output `SelInner` should be placed but match the inner select condition with `m_c_LogicalOp`. With fully simplified ops, this works fine, but its possible if the select condition is not simplified, for it match both `LogicalAnd` and `LogicalOr` i.e `select true, true, false`. In PR71330 for example, the issue occurs in the following IR: ``` define i32 @bad() { %..i.i = select i1 false, i32 0, i32 3 %brmerge = select i1 true, i1 true, i1 false %not.cmp.i.i.not = xor i1 true, true %.mux = zext i1 %not.cmp.i.i.not to i32 %retval.0.i.i = select i1 %brmerge, i32 %.mux, i32 %..i.i ret i32 %retval.0.i.i } ``` When simplifying: ``` %retval.0.i.i = select i1 %brmerge, i32 %.mux, i32 %..i.i ``` We end up matching `%brmerge` as `LogicalAnd` for `IsAndVariant`, but the inner select (`%..i.i`) condition which is `false` with `LogicalOr`.

nikic

LGTM

goldsteinn · 2023-11-09T22:38:57Z

Think this needs to be backported to 17 release. Im checking if the repro still applies. Otherwise will req cherry-pick w.o the test case.

@bad

The bug is that `IsAndVariant` is used to assume which arm in the select the output `SelInner` should be placed but match the inner select condition with `m_c_LogicalOp`. With fully simplified ops, this works fine, but its possible if the select condition is not simplified, for it match both `LogicalAnd` and `LogicalOr` i.e `select true, true, false`. In PR71330 for example, the issue occurs in the following IR: ``` define i32 @bad() { %..i.i = select i1 false, i32 0, i32 3 %brmerge = select i1 true, i1 true, i1 false %not.cmp.i.i.not = xor i1 true, true %.mux = zext i1 %not.cmp.i.i.not to i32 %retval.0.i.i = select i1 %brmerge, i32 %.mux, i32 %..i.i ret i32 %retval.0.i.i } ``` When simplifying: ``` %retval.0.i.i = select i1 %brmerge, i32 %.mux, i32 %..i.i ``` We end up matching `%brmerge` as `LogicalAnd` for `IsAndVariant`, but the inner select (`%..i.i`) condition which is `false` with `LogicalOr`. Closes llvm#71489

goldsteinn requested a review from nikic as a code owner November 7, 2023 05:45

llvmbot added the llvm:transforms label Nov 7, 2023

goldsteinn mentioned this pull request Nov 7, 2023

Wrong code at -O3 on x86_64-linux_gnu since 0289dad (recent regression) #71330

Closed

goldsteinn commented Nov 7, 2023

View reviewed changes

llvm/test/Transforms/InstCombine/pr71330.ll Outdated Show resolved Hide resolved

goldsteinn changed the title ~~goldsteinn/pr71330~~ [InstCombine] Fix buggy transform in foldNestedSelects; PR 71330 Nov 7, 2023

goldsteinn requested a review from dtcxzyw November 7, 2023 05:49

goldsteinn force-pushed the goldsteinn/pr71330 branch from b4b1116 to f58261e Compare November 7, 2023 19:42

goldsteinn commented Nov 7, 2023

View reviewed changes

RKSimon mentioned this pull request Nov 8, 2023

Revert 112e49b38150b8bfdef01434309d1b05204193e4 due to a miscompile #71598

Merged

goldsteinn added 2 commits November 9, 2023 04:53

[InstCombine] Add reduced test for PR 71330; NFC

764cdf8

goldsteinn force-pushed the goldsteinn/pr71330 branch from f58261e to f169726 Compare November 9, 2023 10:54

nikic approved these changes Nov 9, 2023

View reviewed changes

goldsteinn closed this in 9ef8290 Nov 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[InstCombine] Fix buggy transform in foldNestedSelects; PR 71330 #71489

[InstCombine] Fix buggy transform in foldNestedSelects; PR 71330 #71489

Uh oh!

goldsteinn commented Nov 7, 2023

Uh oh!

llvmbot commented Nov 7, 2023

Uh oh!

Uh oh!

goldsteinn Nov 7, 2023

Uh oh!

nikic Nov 7, 2023

Uh oh!

goldsteinn Nov 7, 2023

Uh oh!

goldsteinn Nov 7, 2023 •

edited

Loading

Uh oh!

nikic Nov 8, 2023

Uh oh!

goldsteinn Nov 9, 2023

Uh oh!

nikic left a comment

Uh oh!

goldsteinn commented Nov 9, 2023

Uh oh!

Uh oh!

[InstCombine] Fix buggy transform in foldNestedSelects; PR 71330 #71489

[InstCombine] Fix buggy transform in foldNestedSelects; PR 71330 #71489

Uh oh!

Conversation

goldsteinn commented Nov 7, 2023

Uh oh!

llvmbot commented Nov 7, 2023

Uh oh!

Uh oh!

goldsteinn Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

nikic Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

goldsteinn Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

goldsteinn Nov 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikic Nov 8, 2023

Choose a reason for hiding this comment

Uh oh!

goldsteinn Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

goldsteinn commented Nov 9, 2023

Uh oh!

Uh oh!

goldsteinn Nov 7, 2023 •

edited

Loading