Refactor: Simplify boolean conditional return statements in lib/Transforms/Scalar

[LegalizeAdulthood]

Summary

Use clang-tidy to simplify boolean conditional return statements

[atrick]

I think some of the cases are less readable this way, but don't seriously object. Overall LGTM.

[LegalizeAdulthood]

Update from latest.
Update from comments.
I do not have commit access.

[sanjoy]

I have only one nit inline. Once that's fixed I'm happy to check this in on your behalf.

lib/Transforms/Scalar/EarlyCSE.cpp

218

This one should be "de-morganized" to return CI && CI->onlyReadsMemory();

lib/Transforms/Scalar/EarlyCSE.cpp

struct CallValue {
   }
 
   static bool canHandle(Instruction *Inst) {
     // Don't value number anything that returns void.
     if (Inst->getType()->isVoidTy())
       return false;
 
     CallInst *CI = dyn_cast<CallInst>(Inst);
-    if (!CI || !CI->onlyReadsMemory())
-      return false;
-    return true;
+    return !(!CI || !CI->onlyReadsMemory());
   }
 };
 }
 
 namespace llvm {
 template <> struct DenseMapInfo<CallValue> {
   static inline CallValue getEmptyKey() {
     return DenseMapInfo<Instruction *>::getEmptyKey();

lib/Transforms/Scalar/JumpThreading.cpp

if (getKnownConstant(Condition, Preference)) {
     return true;
   }
 
   Instruction *CondInst = dyn_cast<Instruction>(Condition);
 
   // All the rest of our checks depend on the condition being an instruction.
   if (!CondInst) {
     // FIXME: Unify this with code below.
-    if (ProcessThreadableEdges(Condition, BB, Preference, Terminator))
-      return true;
-    return false;
+    return ProcessThreadableEdges(Condition, BB, Preference, Terminator);
   }
 
 
   if (CmpInst *CondCmp = dyn_cast<CmpInst>(CondInst)) {
     // If we're branching on a conditional, LVI might be able to determine
     // it's value at the branch instruction.  We only handle comparisons
     // against a constant at this time.
     // TODO: This should be extended to handle switches as well.

lib/Transforms/Scalar/LICM.cpp

static bool isGuaranteedToExecute(const Instruction &Inst,
 
   // Verify that the block dominates each of the exit blocks of the loop.
   for (unsigned i = 0, e = ExitBlocks.size(); i != e; ++i)
     if (!DT->dominates(Inst.getParent(), ExitBlocks[i]))
       return false;
 
   // As a degenerate case, if the loop is statically infinite then we haven't
   // proven anything since there are no exit blocks.
-  if (ExitBlocks.empty())
-    return false;
-
-  return true;
+  return !ExitBlocks.empty();
 }
 
 namespace {
   class LoopPromoter : public LoadAndStorePromoter {
     Value *SomePtr;  // Designated pointer to store to.
     SmallPtrSetImpl<Value*> &PointerMustAliases;
     SmallVectorImpl<BasicBlock*> &LoopExitBlocks;
     SmallVectorImpl<Instruction*> &LoopInsertPts;

lib/Transforms/Scalar/LoopIdiomRecognize.cpp

bool LoopIdiomRecognize::processLoopStoreOfLoopLoad(
 
   // Okay, the memcpy has been formed.  Zap the original store and anything that
   // feeds into it.
   deleteDeadInstruction(SI, TLI);
   ++NumMemCpy;
   return true;
 }
 
-bool LoopIdiomRecognize::runOnNoncountableLoop() {
-  if (recognizePopcount())
-    return true;
-
-  return false;
-}
+bool LoopIdiomRecognize::runOnNoncountableLoop() { return recognizePopcount(); }
 
 /// Check if the given conditional branch is based on the comparison between
 /// a variable and zero, and if the variable is non-zero, the control yields to
 /// the loop entry. If the branch matches the behavior, the variable involved
 /// in the comparion is returned. This function will be called to see if the
 /// precondition and postcondition of the loop are in desirable form.
 static Value *matchCondition(BranchInst *BI, BasicBlock *LoopEntry) {
   if (!BI || !BI->isConditional())

lib/Transforms/Scalar/LoopInterchange.cpp

for (J = I, JE = MemInstr.end(); J != JE; ++J) {
             return false;
           }
         }
       }
     }
   }
 
   // We don't have a DepMatrix to check legality return false
-  if (DepMatrix.size() == 0)
-    return false;
-  return true;
+  return DepMatrix.size() != 0;
 }
 
 // A loop is moved from index 'from' to an index 'to'. Update the Dependence
 // matrix by exchanging the two columns.
 static void interChangeDepedencies(CharMatrix &DepMatrix, unsigned FromIndx,
                                    unsigned ToIndx) {
   unsigned numRows = DepMatrix.size();
   for (unsigned i = 0; i < numRows; ++i) {
for (auto I = InnerLoopLatch->rbegin(), E = InnerLoopLatch->rend();
     // safe to split this loop latch.
     if (!Ins.isIdenticalTo(InnerIndexVarInc))
       return true;
     else
       FoundInduction = true;
   }
   // The loop latch ended and we didn't find the induction variable return as
   // current limitation.
-  if (!FoundInduction)
-    return true;
-
-  return false;
+  return !FoundInduction;
 }
 
 bool LoopInterchangeLegality::canInterchangeLoops(unsigned InnerLoopId,
                                                   unsigned OuterLoopId,
                                                   CharMatrix &DepMatrix) {
 
   if (!isLegalToInterChangeLoops(DepMatrix, InnerLoopId, OuterLoopId)) {
     DEBUG(dbgs() << "Failed interchange InnerLoopId = " << InnerLoopId

lib/Transforms/Scalar/LoopStrengthReduce.cpp

static bool canFoldIVIncExpr(const SCEV *IncExpr, Instruction *UserInst,
   if (!IncConst || !isAddressUse(UserInst, Operand))
     return false;
 
   if (IncConst->getValue()->getValue().getMinSignedBits() > 64)
     return false;
 
   MemAccessTy AccessTy = getAccessType(UserInst);
   int64_t IncOffset = IncConst->getValue()->getSExtValue();
-  if (!isAlwaysFoldable(TTI, LSRUse::Address, AccessTy, /*BaseGV=*/nullptr,
-                        IncOffset, /*HaseBaseReg=*/false))
-    return false;
-
-  return true;
+  return isAlwaysFoldable(TTI, LSRUse::Address, AccessTy, /*BaseGV=*/nullptr,
+                          IncOffset, /*HaseBaseReg=*/false);
 }
 
 /// Generate an add or subtract for each IVInc in a chain to materialize the IV
 /// user's operand from the previous IV user's operand.
 void LSRInstance::GenerateIVChain(const IVChain &Chain, SCEVExpander &Rewriter,
                                   SmallVectorImpl<WeakVH> &DeadInsts) {
   // Find the new IVOperand for the head of the chain. It may have been replaced
   // by LSR.

lib/Transforms/Scalar/MergedLoadStoreMotion.cpp

if (!Succ0->getSinglePredecessor() ||
       Succ0->getTerminator()->getNumSuccessors() != 1)
     return false;
   if (!Succ1->getSinglePredecessor() ||
       Succ1->getTerminator()->getNumSuccessors() != 1)
     return false;
 
   BasicBlock *Tail = Succ0->getTerminator()->getSuccessor(0);
   // Ignore triangles.
-  if (Succ1->getTerminator()->getSuccessor(0) != Tail)
-    return false;
-  return true;
+  return Succ1->getTerminator()->getSuccessor(0) == Tail;
 }
 
 ///
 /// \brief True when instruction is a hoist barrier for a load
 ///
 /// Whenever an instruction could possibly modify the value
 /// being loaded or protect against the load from happening
 /// it is considered a hoist barrier.

lib/Transforms/Scalar/Reassociate.cpp

static bool ShouldBreakUpSubtract(Instruction *Sub) {
   if (isReassociableOp(V0, Instruction::Add, Instruction::FAdd) ||
       isReassociableOp(V0, Instruction::Sub, Instruction::FSub))
     return true;
   Value *V1 = Sub->getOperand(1);
   if (isReassociableOp(V1, Instruction::Add, Instruction::FAdd) ||
       isReassociableOp(V1, Instruction::Sub, Instruction::FSub))
     return true;
   Value *VB = Sub->user_back();
-  if (Sub->hasOneUse() &&
+  return Sub->hasOneUse() &&
          (isReassociableOp(VB, Instruction::Add, Instruction::FAdd) ||
-       isReassociableOp(VB, Instruction::Sub, Instruction::FSub)))
-    return true;
-
-  return false;
+          isReassociableOp(VB, Instruction::Sub, Instruction::FSub));
 }
 
 /// If we have (X-Y), and if either X is an add, or if this is only used by an
 /// add, transform this into (X+(0-Y)) to promote better reassociation.
 static BinaryOperator *BreakUpSubtract(Instruction *Sub) {
   // Convert a subtract into an add and a neg instruction. This allows sub
   // instructions to be commuted with other add instructions.
   //

lib/Transforms/Scalar/ScalarReplAggregates.cpp

 /// isCompatibleAggregate - Check if T1 and T2 are either the same type or are
 /// "homogeneous" aggregates with the same element type and number of elements.
 static bool isCompatibleAggregate(Type *T1, Type *T2) {
   if (T1 == T2)
     return true;
 
   unsigned NumElts1, NumElts2;
   Type *EltTy1, *EltTy2;
-  if (isHomogeneousAggregate(T1, NumElts1, EltTy1) &&
-      isHomogeneousAggregate(T2, NumElts2, EltTy2) &&
-      NumElts1 == NumElts2 &&
-      EltTy1 == EltTy2)
-    return true;
-
-  return false;
+  return isHomogeneousAggregate(T1, NumElts1, EltTy1) &&
+         isHomogeneousAggregate(T2, NumElts2, EltTy2) && NumElts1 == NumElts2 &&
+         EltTy1 == EltTy2;
 }
 
 /// isSafeMemAccess - Check if a load/store/memcpy operates on the entire AI
 /// alloca or has an offset and size that corresponds to a component element
 /// within it.  The offset checked here may have been formed from a GEP with a
 /// pointer bitcasted to a different type.
 ///
 /// If AllowWholeAccess is true, then this allows uses of the entire alloca as a