From 0b4a709cf73c6071de118b7b0af099454d63d213 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 14 Apr 2017 19:20:12 +0000 Subject: [PATCH] [InstCombine] Support folding a subtract with a constant LHS into a phi node We currently only support folding a subtract into a select but not a PHI. This fixes that. I had to fix an assumption in FoldOpIntoPhi that assumed the PHI node was always in operand 0. Now we pass it in like we do for FoldOpIntoSelect. But we still require some dancing to find the Constant when we create the BinOp or ConstantExpr. This is based code is similar to what we do for selects. Since I touched all call sites, this also renames FoldOpIntoPhi to foldOpIntoPhi to match coding standards. Differential Revision: https://reviews.llvm.org/D31686 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@300363 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/InstCombine/InstCombineAddSub.cpp | 5 +++ lib/Transforms/InstCombine/InstCombineCasts.cpp | 4 +- lib/Transforms/InstCombine/InstCombineCompares.cpp | 4 +- lib/Transforms/InstCombine/InstCombineInternal.h | 2 +- .../InstCombine/InstCombineMulDivRem.cpp | 6 +-- lib/Transforms/InstCombine/InstCombinePHI.cpp | 4 +- lib/Transforms/InstCombine/InstCombineSelect.cpp | 4 +- .../InstCombine/InstructionCombining.cpp | 43 ++++++++++++++-------- test/Transforms/InstCombine/sub.ll | 15 +++----- 9 files changed, 50 insertions(+), 37 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/lib/Transforms/InstCombine/InstCombineAddSub.cpp index 1077121f8cb..174ec803627 100644 --- a/lib/Transforms/InstCombine/InstCombineAddSub.cpp +++ b/lib/Transforms/InstCombine/InstCombineAddSub.cpp @@ -1572,6 +1572,11 @@ Instruction *InstCombiner::visitSub(BinaryOperator &I) { if (Instruction *R = FoldOpIntoSelect(I, SI)) return R; + // Try to fold constant sub into PHI values. + if (PHINode *PN = dyn_cast(Op1)) + if (Instruction *R = foldOpIntoPhi(I, PN)) + return R; + // C-(X+C2) --> (C-C2)-X Constant *C2; if (match(Op1, m_Add(m_Value(X), m_Constant(C2)))) diff --git a/lib/Transforms/InstCombine/InstCombineCasts.cpp b/lib/Transforms/InstCombine/InstCombineCasts.cpp index e08c301ccdd..25683132c78 100644 --- a/lib/Transforms/InstCombine/InstCombineCasts.cpp +++ b/lib/Transforms/InstCombine/InstCombineCasts.cpp @@ -274,12 +274,12 @@ Instruction *InstCombiner::commonCastTransforms(CastInst &CI) { return NV; // If we are casting a PHI, then fold the cast into the PHI. - if (isa(Src)) { + if (auto *PN = dyn_cast(Src)) { // Don't do this if it would create a PHI node with an illegal type from a // legal type. if (!Src->getType()->isIntegerTy() || !CI.getType()->isIntegerTy() || shouldChangeType(CI.getType(), Src->getType())) - if (Instruction *NV = FoldOpIntoPhi(CI)) + if (Instruction *NV = foldOpIntoPhi(CI, PN)) return NV; } diff --git a/lib/Transforms/InstCombine/InstCombineCompares.cpp b/lib/Transforms/InstCombine/InstCombineCompares.cpp index 5a292ef849e..bbafa9e9f46 100644 --- a/lib/Transforms/InstCombine/InstCombineCompares.cpp +++ b/lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -2709,7 +2709,7 @@ Instruction *InstCombiner::foldICmpInstWithConstantNotInt(ICmpInst &I) { // block. If in the same block, we're encouraging jump threading. If // not, we are just pessimizing the code by making an i1 phi. if (LHSI->getParent() == I.getParent()) - if (Instruction *NV = FoldOpIntoPhi(I)) + if (Instruction *NV = foldOpIntoPhi(I, cast(LHSI))) return NV; break; case Instruction::Select: { @@ -4873,7 +4873,7 @@ Instruction *InstCombiner::visitFCmpInst(FCmpInst &I) { // block. If in the same block, we're encouraging jump threading. If // not, we are just pessimizing the code by making an i1 phi. if (LHSI->getParent() == I.getParent()) - if (Instruction *NV = FoldOpIntoPhi(I)) + if (Instruction *NV = foldOpIntoPhi(I, cast(LHSI))) return NV; break; case Instruction::SIToFP: diff --git a/lib/Transforms/InstCombine/InstCombineInternal.h b/lib/Transforms/InstCombine/InstCombineInternal.h index f163f57f54b..71000063ab3 100644 --- a/lib/Transforms/InstCombine/InstCombineInternal.h +++ b/lib/Transforms/InstCombine/InstCombineInternal.h @@ -569,7 +569,7 @@ private: /// Given a binary operator, cast instruction, or select which has a PHI node /// as operand #0, see if we can fold the instruction into the PHI (which is /// only possible if all operands to the PHI are constants). - Instruction *FoldOpIntoPhi(Instruction &I); + Instruction *foldOpIntoPhi(Instruction &I, PHINode *PN); /// Given an instruction with a select as one operand and a constant as the /// other operand, try to fold the binary operator into the select arguments. diff --git a/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp index a238f3f0505..f1ac82057e6 100644 --- a/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp +++ b/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp @@ -1455,16 +1455,16 @@ Instruction *InstCombiner::commonIRemTransforms(BinaryOperator &I) { if (SelectInst *SI = dyn_cast(Op0I)) { if (Instruction *R = FoldOpIntoSelect(I, SI)) return R; - } else if (isa(Op0I)) { + } else if (auto *PN = dyn_cast(Op0I)) { using namespace llvm::PatternMatch; const APInt *Op1Int; if (match(Op1, m_APInt(Op1Int)) && !Op1Int->isMinValue() && (I.getOpcode() == Instruction::URem || !Op1Int->isMinSignedValue())) { - // FoldOpIntoPhi will speculate instructions to the end of the PHI's + // foldOpIntoPhi will speculate instructions to the end of the PHI's // predecessor blocks, so do this only if we know the srem or urem // will not fault. - if (Instruction *NV = FoldOpIntoPhi(I)) + if (Instruction *NV = foldOpIntoPhi(I, PN)) return NV; } } diff --git a/lib/Transforms/InstCombine/InstCombinePHI.cpp b/lib/Transforms/InstCombine/InstCombinePHI.cpp index d8574175307..85e5b6ba2dc 100644 --- a/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -457,8 +457,8 @@ Instruction *InstCombiner::FoldPHIArgZextsIntoPHI(PHINode &Phi) { } // The more common cases of a phi with no constant operands or just one - // variable operand are handled by FoldPHIArgOpIntoPHI() and FoldOpIntoPhi() - // respectively. FoldOpIntoPhi() wants to do the opposite transform that is + // variable operand are handled by FoldPHIArgOpIntoPHI() and foldOpIntoPhi() + // respectively. foldOpIntoPhi() wants to do the opposite transform that is // performed here. It tries to replicate a cast in the phi operand's basic // block to expose other folding opportunities. Thus, InstCombine will // infinite loop without this check. diff --git a/lib/Transforms/InstCombine/InstCombineSelect.cpp b/lib/Transforms/InstCombine/InstCombineSelect.cpp index b7099bec380..693b6c95c16 100644 --- a/lib/Transforms/InstCombine/InstCombineSelect.cpp +++ b/lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -1384,11 +1384,11 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) { } // See if we can fold the select into a phi node if the condition is a select. - if (isa(SI.getCondition())) + if (auto *PN = dyn_cast(SI.getCondition())) // The true/false values have to be live in the PHI predecessor's blocks. if (canSelectOperandBeMappingIntoPredBlock(TrueVal, SI) && canSelectOperandBeMappingIntoPredBlock(FalseVal, SI)) - if (Instruction *NV = FoldOpIntoPhi(SI)) + if (Instruction *NV = foldOpIntoPhi(SI, PN)) return NV; if (SelectInst *TrueSI = dyn_cast(TrueVal)) { diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp index 223fa2adc0c..88ef17bbc8f 100644 --- a/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -839,8 +839,29 @@ Instruction *InstCombiner::FoldOpIntoSelect(Instruction &Op, SelectInst *SI) { return SelectInst::Create(SI->getCondition(), NewTV, NewFV, "", nullptr, SI); } -Instruction *InstCombiner::FoldOpIntoPhi(Instruction &I) { - PHINode *PN = cast(I.getOperand(0)); +static Value *foldOperationIntoPhiValue(BinaryOperator *I, Value *InV, + InstCombiner *IC) { + bool ConstIsRHS = isa(I->getOperand(1)); + Constant *C = cast(I->getOperand(ConstIsRHS)); + + if (auto *InC = dyn_cast(InV)) { + if (ConstIsRHS) + return ConstantExpr::get(I->getOpcode(), InC, C); + return ConstantExpr::get(I->getOpcode(), C, InC); + } + + Value *Op0 = InV, *Op1 = C; + if (!ConstIsRHS) + std::swap(Op0, Op1); + + Value *RI = IC->Builder->CreateBinOp(I->getOpcode(), Op0, Op1, "phitmp"); + auto *FPInst = dyn_cast(RI); + if (FPInst && isa(FPInst)) + FPInst->copyFastMathFlags(I); + return RI; +} + +Instruction *InstCombiner::foldOpIntoPhi(Instruction &I, PHINode *PN) { unsigned NumPHIValues = PN->getNumIncomingValues(); if (NumPHIValues == 0) return nullptr; @@ -946,19 +967,9 @@ Instruction *InstCombiner::FoldOpIntoPhi(Instruction &I) { C, "phitmp"); NewPN->addIncoming(InV, PN->getIncomingBlock(i)); } - } else if (I.getNumOperands() == 2) { - Constant *C = cast(I.getOperand(1)); + } else if (auto *BO = dyn_cast(&I)) { for (unsigned i = 0; i != NumPHIValues; ++i) { - Value *InV = nullptr; - if (Constant *InC = dyn_cast(PN->getIncomingValue(i))) { - InV = ConstantExpr::get(I.getOpcode(), InC, C); - } else { - InV = Builder->CreateBinOp(cast(I).getOpcode(), - PN->getIncomingValue(i), C, "phitmp"); - auto *FPInst = dyn_cast(InV); - if (FPInst && isa(FPInst)) - FPInst->copyFastMathFlags(&I); - } + Value *InV = foldOperationIntoPhiValue(BO, PN->getIncomingValue(i), this); NewPN->addIncoming(InV, PN->getIncomingBlock(i)); } } else { @@ -990,8 +1001,8 @@ Instruction *InstCombiner::foldOpWithConstantIntoOperand(BinaryOperator &I) { if (auto *Sel = dyn_cast(I.getOperand(0))) { if (Instruction *NewSel = FoldOpIntoSelect(I, Sel)) return NewSel; - } else if (isa(I.getOperand(0))) { - if (Instruction *NewPhi = FoldOpIntoPhi(I)) + } else if (auto *PN = dyn_cast(I.getOperand(0))) { + if (Instruction *NewPhi = foldOpIntoPhi(I, PN)) return NewPhi; } return nullptr; diff --git a/test/Transforms/InstCombine/sub.ll b/test/Transforms/InstCombine/sub.ll index 3b3a995c43d..2388301c726 100644 --- a/test/Transforms/InstCombine/sub.ll +++ b/test/Transforms/InstCombine/sub.ll @@ -913,9 +913,8 @@ define i32 @test55(i1 %which) { ; CHECK: delay: ; CHECK-NEXT: br label [[FINAL]] ; CHECK: final: -; CHECK-NEXT: [[A:%.*]] = phi i32 [ 1000, [[ENTRY:%.*]] ], [ 10, [[DELAY]] ] -; CHECK-NEXT: [[VALUE:%.*]] = sub nsw i32 123, [[A]] -; CHECK-NEXT: ret i32 [[VALUE]] +; CHECK-NEXT: [[A:%.*]] = phi i32 [ -877, [[ENTRY:%.*]] ], [ 113, [[DELAY]] ] +; CHECK-NEXT: ret i32 [[A]] ; entry: br i1 %which, label %final, label %delay @@ -936,9 +935,8 @@ define <2 x i32> @test55vec(i1 %which) { ; CHECK: delay: ; CHECK-NEXT: br label [[FINAL]] ; CHECK: final: -; CHECK-NEXT: [[A:%.*]] = phi <2 x i32> [ , [[ENTRY:%.*]] ], [ , [[DELAY]] ] -; CHECK-NEXT: [[VALUE:%.*]] = sub nsw <2 x i32> , [[A]] -; CHECK-NEXT: ret <2 x i32> [[VALUE]] +; CHECK-NEXT: [[A:%.*]] = phi <2 x i32> [ , [[ENTRY:%.*]] ], [ , [[DELAY]] ] +; CHECK-NEXT: ret <2 x i32> [[A]] ; entry: br i1 %which, label %final, label %delay @@ -959,9 +957,8 @@ define <2 x i32> @test55vec2(i1 %which) { ; CHECK: delay: ; CHECK-NEXT: br label [[FINAL]] ; CHECK: final: -; CHECK-NEXT: [[A:%.*]] = phi <2 x i32> [ , [[ENTRY:%.*]] ], [ , [[DELAY]] ] -; CHECK-NEXT: [[VALUE:%.*]] = sub nsw <2 x i32> , [[A]] -; CHECK-NEXT: ret <2 x i32> [[VALUE]] +; CHECK-NEXT: [[A:%.*]] = phi <2 x i32> [ , [[ENTRY:%.*]] ], [ , [[DELAY]] ] +; CHECK-NEXT: ret <2 x i32> [[A]] ; entry: br i1 %which, label %final, label %delay -- 2.11.0