From 4f6fd5fe68221ad9a57b8b1156a459951db66d89 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Fri, 6 Oct 2017 23:20:16 +0000 Subject: [PATCH] [InstCombine] rename SimplifyDivRemOfSelect to be clearer, add comments, simplify code; NFCI There's at least one bug here - this code can fail with vector types (PR34856). It's also being called for FREM; I'm still trying to understand how that is valid. llvm-svn: 315127 --- .../InstCombine/InstCombineInternal.h | 2 +- .../InstCombine/InstCombineMulDivRem.cpp | 38 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineInternal.h b/lib/Transforms/InstCombine/InstCombineInternal.h index 6b7bcd97e8f..46f86114718 100644 --- a/lib/Transforms/InstCombine/InstCombineInternal.h +++ b/lib/Transforms/InstCombine/InstCombineInternal.h @@ -277,7 +277,7 @@ public: Instruction *visitURem(BinaryOperator &I); Instruction *visitSRem(BinaryOperator &I); Instruction *visitFRem(BinaryOperator &I); - bool SimplifyDivRemOfSelect(BinaryOperator &I); + bool simplifyDivRemOfSelectWithZeroOp(BinaryOperator &I); Instruction *commonRemTransforms(BinaryOperator &I); Instruction *commonIRemTransforms(BinaryOperator &I); Instruction *commonDivTransforms(BinaryOperator &I); diff --git a/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp index 0f762710fde..4290d8db989 100644 --- a/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp +++ b/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp @@ -778,24 +778,23 @@ Instruction *InstCombiner::visitFMul(BinaryOperator &I) { return Changed ? &I : nullptr; } -/// Try to fold a divide or remainder of a select instruction. -bool InstCombiner::SimplifyDivRemOfSelect(BinaryOperator &I) { - SelectInst *SI = cast(I.getOperand(1)); - - // div/rem X, (Cond ? 0 : Y) -> div/rem X, Y - int NonNullOperand = -1; - if (Constant *ST = dyn_cast(SI->getOperand(1))) - if (ST->isNullValue()) - NonNullOperand = 2; - // div/rem X, (Cond ? Y : 0) -> div/rem X, Y - if (Constant *ST = dyn_cast(SI->getOperand(2))) - if (ST->isNullValue()) - NonNullOperand = 1; - - if (NonNullOperand == -1) +/// Fold a divide or remainder with a select instruction divisor when one of the +/// select operands is zero. In that case, we can use the other select operand +/// because div/rem by zero is undefined. +bool InstCombiner::simplifyDivRemOfSelectWithZeroOp(BinaryOperator &I) { + SelectInst *SI = dyn_cast(I.getOperand(1)); + if (!SI) return false; - Value *SelectCond = SI->getOperand(0); + int NonNullOperand; + if (match(SI->getTrueValue(), m_Zero())) + // div/rem X, (Cond ? 0 : Y) -> div/rem X, Y + NonNullOperand = 2; + else if (match(SI->getFalseValue(), m_Zero())) + // div/rem X, (Cond ? Y : 0) -> div/rem X, Y + NonNullOperand = 1; + else + return false; // Change the div/rem to use 'Y' instead of the select. I.setOperand(1, SI->getOperand(NonNullOperand)); @@ -808,6 +807,7 @@ bool InstCombiner::SimplifyDivRemOfSelect(BinaryOperator &I) { // If the select and condition only have a single use, don't bother with this, // early exit. + Value *SelectCond = SI->getCondition(); if (SI->use_empty() && SelectCond->hasOneUse()) return true; @@ -863,7 +863,7 @@ Instruction *InstCombiner::commonIDivTransforms(BinaryOperator &I) { // Handle cases involving: [su]div X, (select Cond, Y, Z) // This does not apply for fdiv. - if (isa(Op1) && SimplifyDivRemOfSelect(I)) + if (simplifyDivRemOfSelectWithZeroOp(I)) return &I; if (Instruction *LHS = dyn_cast(Op0)) { @@ -1458,7 +1458,7 @@ Instruction *InstCombiner::commonIRemTransforms(BinaryOperator &I) { } // Handle cases involving: rem X, (select Cond, Y, Z) - if (isa(Op1) && SimplifyDivRemOfSelect(I)) + if (simplifyDivRemOfSelectWithZeroOp(I)) return &I; if (isa(Op1)) { @@ -1613,7 +1613,7 @@ Instruction *InstCombiner::visitFRem(BinaryOperator &I) { return replaceInstUsesWith(I, V); // Handle cases involving: rem X, (select Cond, Y, Z) - if (isa(Op1) && SimplifyDivRemOfSelect(I)) + if (simplifyDivRemOfSelectWithZeroOp(I)) return &I; return nullptr;