From 1f8ac37e2d50b0175225a0d8f3f5003f64b46b89 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Mon, 31 Oct 2022 17:32:34 -0400 Subject: [PATCH] [InstCombine] adjust branch on logical-and fold The transform was just added with: 115d2f69a515cd756fa51 ...but as noted in post-commit feedback, it was confusingly coded. Now, we create the final expected canonicalized form directly and put an extra use check on the match, so we should not ever end up with more instructions. --- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 12 ++++++++---- llvm/test/Transforms/InstCombine/branch.ll | 7 ++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 4459e16..ba51ee5 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -3187,13 +3187,17 @@ Instruction *InstCombinerImpl::visitBranchInst(BranchInst &BI) { return replaceOperand(BI, 0, X); } - // br (X && !Y), T, F --> br ((X && Y) || !X), F, T + // Canonicalize logical-and-with-invert as logical-or-with-invert. + // This is done by inverting the condition and swapping successors: + // br (X && !Y), T, F --> br !(X && !Y), F, T --> br (!X || Y), F, T Value *Y; if (isa(Cond) && - match(Cond, m_OneUse(m_LogicalAnd(m_Value(X), m_Not(m_Value(Y)))))) { - Value *AndOr = Builder.CreateSelect(X, Y, Builder.getTrue()); + match(Cond, + m_OneUse(m_LogicalAnd(m_Value(X), m_OneUse(m_Not(m_Value(Y))))))) { + Value *NotX = Builder.CreateNot(X, "not." + X->getName()); + Value *Or = Builder.CreateLogicalOr(NotX, Y); BI.swapSuccessors(); - return replaceOperand(BI, 0, AndOr); + return replaceOperand(BI, 0, Or); } // If the condition is irrelevant, remove the use so that other diff --git a/llvm/test/Transforms/InstCombine/branch.ll b/llvm/test/Transforms/InstCombine/branch.ll index 2961dec..1110d5f 100644 --- a/llvm/test/Transforms/InstCombine/branch.ll +++ b/llvm/test/Transforms/InstCombine/branch.ll @@ -189,14 +189,15 @@ f: ret i32 3 } +; negative test + define i32 @logical_and_not_use1(i1 %x, i1 %y) { ; CHECK-LABEL: @logical_and_not_use1( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[NOTY:%.*]] = xor i1 [[Y:%.*]], true ; CHECK-NEXT: call void @use(i1 [[NOTY]]) -; CHECK-NEXT: [[NOT_X:%.*]] = xor i1 [[X:%.*]], true -; CHECK-NEXT: [[TMP0:%.*]] = select i1 [[NOT_X]], i1 true, i1 [[Y]] -; CHECK-NEXT: br i1 [[TMP0]], label [[F:%.*]], label [[T:%.*]] +; CHECK-NEXT: [[AND:%.*]] = select i1 [[X:%.*]], i1 [[NOTY]], i1 false +; CHECK-NEXT: br i1 [[AND]], label [[T:%.*]], label [[F:%.*]] ; CHECK: t: ; CHECK-NEXT: ret i32 42 ; CHECK: f: -- 2.7.4