From 337948ac6e2260fc4d5a1901b4f667a2a0a52ee3 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Sat, 20 Nov 2021 10:55:41 -0500 Subject: [PATCH] [InstCombine] add folds for binop with sexted bool and constant operands This is a generalization/extension of the existing and/or folds noted with TODO comments. Those have a one-use constraint that is not necessary. Potential follow-ups are noted by the TODO comments in the new function. We can also call this function from other binop visit* functions, but we need to add tests first. This solves: https://llvm.org/PR52543 https://alive2.llvm.org/ce/z/NWuCR5 --- .../InstCombine/InstCombineAndOrXor.cpp | 14 +++++++++++ .../InstCombine/InstCombineInternal.h | 1 + .../InstCombine/InstructionCombining.cpp | 23 +++++++++++++++++++ llvm/test/Transforms/InstCombine/and.ll | 2 +- llvm/test/Transforms/InstCombine/or.ll | 2 +- 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp index 06c9bf650f37..4ef7ff31e089 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp @@ -2061,7 +2061,14 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) { if (Instruction *CastedAnd = foldCastedBitwiseLogic(I)) return CastedAnd; + if (Instruction *Sel = foldBinopOfSextBoolToSelect(I)) + return Sel; + // and(sext(A), B) / and(B, sext(A)) --> A ? B : 0, where A is i1 or . + // TODO: Move this into foldBinopOfSextBoolToSelect as a more generalized fold + // with binop identity constant. But creating a select with non-constant + // arm may not be reversible due to poison semantics. Is that a good + // canonicalization? Value *A; if (match(Op0, m_OneUse(m_SExt(m_Value(A)))) && A->getType()->isIntOrIntVectorTy(1)) @@ -2788,7 +2795,14 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) { if (Instruction *CastedOr = foldCastedBitwiseLogic(I)) return CastedOr; + if (Instruction *Sel = foldBinopOfSextBoolToSelect(I)) + return Sel; + // or(sext(A), B) / or(B, sext(A)) --> A ? -1 : B, where A is i1 or . + // TODO: Move this into foldBinopOfSextBoolToSelect as a more generalized fold + // with binop identity constant. But creating a select with non-constant + // arm may not be reversible due to poison semantics. Is that a good + // canonicalization? if (match(Op0, m_OneUse(m_SExt(m_Value(A)))) && A->getType()->isIntOrIntVectorTy(1)) return SelectInst::Create(A, ConstantInt::getSigned(I.getType(), -1), Op1); diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h index 72e1b21e8d49..20c75188ec9f 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h +++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h @@ -319,6 +319,7 @@ private: Instruction *scalarizePHI(ExtractElementInst &EI, PHINode *PN); Instruction *foldBitcastExtElt(ExtractElementInst &ExtElt); Instruction *foldCastedBitwiseLogic(BinaryOperator &I); + Instruction *foldBinopOfSextBoolToSelect(BinaryOperator &I); Instruction *narrowBinOp(TruncInst &Trunc); Instruction *narrowMaskedBinOp(BinaryOperator &And); Instruction *narrowMathIfNoOverflow(BinaryOperator &I); diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 47b6dcb67a78..1f81624f79e7 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -967,6 +967,29 @@ Value *InstCombinerImpl::dyn_castNegVal(Value *V) const { return nullptr; } +/// A binop with a constant operand and a sign-extended boolean operand may be +/// converted into a select of constants by applying the binary operation to +/// the constant with the two possible values of the extended boolean (0 or -1). +Instruction *InstCombinerImpl::foldBinopOfSextBoolToSelect(BinaryOperator &BO) { + // TODO: Handle non-commutative binop (constant is operand 0). + // TODO: Handle zext. + // TODO: Peek through 'not' of cast. + Value *BO0 = BO.getOperand(0); + Value *BO1 = BO.getOperand(1); + Value *X; + Constant *C; + if (!match(BO0, m_SExt(m_Value(X))) || !match(BO1, m_ImmConstant(C)) || + !X->getType()->isIntOrIntVectorTy(1)) + return nullptr; + + // bo (sext i1 X), C --> select X, (bo -1, C), (bo 0, C) + Constant *Ones = ConstantInt::getAllOnesValue(BO.getType()); + Constant *Zero = ConstantInt::getNullValue(BO.getType()); + Constant *TVal = ConstantExpr::get(BO.getOpcode(), Ones, C); + Constant *FVal = ConstantExpr::get(BO.getOpcode(), Zero, C); + return SelectInst::Create(X, TVal, FVal); +} + static Value *foldOperationIntoSelectOperand(Instruction &I, Value *SO, InstCombiner::BuilderTy &Builder) { if (auto *Cast = dyn_cast(&I)) diff --git a/llvm/test/Transforms/InstCombine/and.ll b/llvm/test/Transforms/InstCombine/and.ll index ec16ffc36106..c5b624bb9933 100644 --- a/llvm/test/Transforms/InstCombine/and.ll +++ b/llvm/test/Transforms/InstCombine/and.ll @@ -1510,7 +1510,7 @@ define i32 @sext_to_sel_multi_use_constant_mask(i1 %y) { ; CHECK-LABEL: @sext_to_sel_multi_use_constant_mask( ; CHECK-NEXT: [[SEXT:%.*]] = sext i1 [[Y:%.*]] to i32 ; CHECK-NEXT: call void @use32(i32 [[SEXT]]) -; CHECK-NEXT: [[R:%.*]] = and i32 [[SEXT]], 42 +; CHECK-NEXT: [[R:%.*]] = select i1 [[Y]], i32 42, i32 0 ; CHECK-NEXT: ret i32 [[R]] ; %sext = sext i1 %y to i32 diff --git a/llvm/test/Transforms/InstCombine/or.ll b/llvm/test/Transforms/InstCombine/or.ll index 64266124aab3..376ae78055ee 100644 --- a/llvm/test/Transforms/InstCombine/or.ll +++ b/llvm/test/Transforms/InstCombine/or.ll @@ -657,7 +657,7 @@ define i32 @sext_to_sel_multi_use_constant_mask(i1 %y) { ; CHECK-LABEL: @sext_to_sel_multi_use_constant_mask( ; CHECK-NEXT: [[SEXT:%.*]] = sext i1 [[Y:%.*]] to i32 ; CHECK-NEXT: call void @use(i32 [[SEXT]]) -; CHECK-NEXT: [[R:%.*]] = or i32 [[SEXT]], 42 +; CHECK-NEXT: [[R:%.*]] = select i1 [[Y]], i32 -1, i32 42 ; CHECK-NEXT: ret i32 [[R]] ; %sext = sext i1 %y to i32