Index: lib/Transforms/Scalar/NewGVN.cpp =================================================================== --- lib/Transforms/Scalar/NewGVN.cpp +++ lib/Transforms/Scalar/NewGVN.cpp @@ -1660,34 +1660,34 @@ LLVM_DEBUG(dbgs() << *NewClass->getMemoryLeader() << "\n"); auto LookupResult = MemoryAccessToClass.find(From); + assert(LookupResult != MemoryAccessToClass.end() && + "MemoryAccess not in MemoryAccessToClass table"); + bool Changed = false; - // If it's already in the table, see if the value changed. - if (LookupResult != MemoryAccessToClass.end()) { - auto *OldClass = LookupResult->second; - if (OldClass != NewClass) { - // If this is a phi, we have to handle memory member updates. - if (auto *MP = dyn_cast(From)) { - OldClass->memory_erase(MP); - NewClass->memory_insert(MP); - // This may have killed the class if it had no non-memory members - if (OldClass->getMemoryLeader() == From) { - if (OldClass->definesNoMemory()) { - OldClass->setMemoryLeader(nullptr); - } else { - OldClass->setMemoryLeader(getNextMemoryLeader(OldClass)); - LLVM_DEBUG(dbgs() << "Memory class leader change for class " - << OldClass->getID() << " to " - << *OldClass->getMemoryLeader() - << " due to removal of a memory member " << *From - << "\n"); - markMemoryLeaderChangeTouched(OldClass); - } + auto *OldClass = LookupResult->second; + if (OldClass != NewClass) { + // If this is a phi, we have to handle memory member updates. + if (auto *MP = dyn_cast(From)) { + OldClass->memory_erase(MP); + NewClass->memory_insert(MP); + // This may have killed the class if it had no non-memory members + if (OldClass->getMemoryLeader() == From) { + if (OldClass->definesNoMemory()) { + OldClass->setMemoryLeader(nullptr); + } else { + OldClass->setMemoryLeader(getNextMemoryLeader(OldClass)); + LLVM_DEBUG(dbgs() << "Memory class leader change for class " + << OldClass->getID() << " to " + << *OldClass->getMemoryLeader() + << " due to removal of a memory member " << *From + << "\n"); + markMemoryLeaderChangeTouched(OldClass); } } - // It wasn't equivalent before, and now it is. - LookupResult->second = NewClass; - Changed = true; } + // It wasn't equivalent before, and now it is. + LookupResult->second = NewClass; + Changed = true; } return Changed; @@ -1758,6 +1758,7 @@ LLVM_DEBUG( dbgs() << "PHI Node " << *I << " has no non-undef arguments, valuing it as undef\n"); + deleteExpression(E); return createConstantExpression(UndefValue::get(I->getType())); } @@ -1892,15 +1893,14 @@ // icmp slt %c, %b.0 // %c and %a may start out equal, and thus, the code below will say the second - // %icmp is false. c may become equal to something else, and in that case the - // %second icmp *must* be reexamined, but would not if only the renamed - // %operands are considered users of the icmp. + // icmp is false. c may become equal to something else, and in that case the + // second icmp *must* be reexamined, but would not if only the renamed + // operands are considered users of the icmp. // *Currently* we only check one level of comparisons back, and only mark one // level back as touched when changes happen. If you modify this code to look // back farther through comparisons, you *must* mark the appropriate - // comparisons as users in PredicateInfo.cpp, or you will cause bugs. See if - // we know something just from the operands themselves + // comparisons as users in PredicateInfo.cpp, or you will cause bugs. // See if our operands have predicate info, so that we may be able to derive // something from a previous comparison. @@ -1914,9 +1914,6 @@ // in a different context. if (!DT->dominates(PBranch->To, getBlockForValue(I))) continue; - // TODO: Along the false edge, we may know more things too, like - // icmp of - // same operands is false. // TODO: We only handle actual comparison conditions below, not // and/or. auto *BranchCond = dyn_cast(PBranch->Condition); @@ -1930,37 +1927,23 @@ BranchPredicate = BranchCond->getSwappedPredicate(); } if (BranchOp0 == Op0 && BranchOp1 == Op1) { - if (PBranch->TrueEdge) { - // If we know the previous predicate is true and we are in the true - // edge then we may be implied true or false. - if (CmpInst::isImpliedTrueByMatchingCmp(BranchPredicate, - OurPredicate)) { - addPredicateUsers(PI, I); - return createConstantExpression( - ConstantInt::getTrue(CI->getType())); - } + if (!PBranch->TrueEdge) { + BranchPredicate = CmpInst::getInversePredicate(BranchPredicate); + } - if (CmpInst::isImpliedFalseByMatchingCmp(BranchPredicate, - OurPredicate)) { - addPredicateUsers(PI, I); - return createConstantExpression( - ConstantInt::getFalse(CI->getType())); - } - } else { - // Just handle the ne and eq cases, where if we have the same - // operands, we may know something. - if (BranchPredicate == OurPredicate) { - addPredicateUsers(PI, I); - // Same predicate, same ops,we know it was false, so this is false. - return createConstantExpression( - ConstantInt::getFalse(CI->getType())); - } else if (BranchPredicate == - CmpInst::getInversePredicate(OurPredicate)) { - addPredicateUsers(PI, I); - // Inverse predicate, we know the other was false, so this is true. - return createConstantExpression( - ConstantInt::getTrue(CI->getType())); - } + // Check if one predicate implies anything about the other. + if (CmpInst::isImpliedTrueByMatchingCmp(BranchPredicate, + OurPredicate)) { + addPredicateUsers(PI, I); + return createConstantExpression( + ConstantInt::getTrue(CI->getType())); + } + + if (CmpInst::isImpliedFalseByMatchingCmp(BranchPredicate, + OurPredicate)) { + addPredicateUsers(PI, I); + return createConstantExpression( + ConstantInt::getFalse(CI->getType())); } } } @@ -2118,7 +2101,7 @@ if (auto *PBranch = dyn_cast(PB)) PredicateToUsers[PBranch->Condition].insert(I); - else if (auto *PAssume = dyn_cast(PB)) + else if (auto *PAssume = dyn_cast(PB)) PredicateToUsers[PAssume->Condition].insert(I); } @@ -2517,33 +2500,17 @@ updateReachableEdge(B, FalseSucc); } } else if (auto *SI = dyn_cast(TI)) { - // For switches, propagate the case values into the case - // destinations. - - // Remember how many outgoing edges there are to every successor. - SmallDenseMap SwitchEdges; - Value *SwitchCond = SI->getCondition(); Value *CondEvaluated = findConditionEquivalence(SwitchCond); - // See if we were able to turn this switch statement into a constant. if (CondEvaluated && isa(CondEvaluated)) { + // If we know the switch target, mark it as reachable auto *CondVal = cast(CondEvaluated); - // We should be able to get case value for this. auto Case = *SI->findCaseValue(CondVal); - if (Case.getCaseSuccessor() == SI->getDefaultDest()) { - // We proved the value is outside of the range of the case. - // We can't do anything other than mark the default dest as reachable, - // and go home. - updateReachableEdge(B, SI->getDefaultDest()); - return; - } - // Now get where it goes and mark it reachable. BasicBlock *TargetBlock = Case.getCaseSuccessor(); updateReachableEdge(B, TargetBlock); } else { for (unsigned i = 0, e = SI->getNumSuccessors(); i != e; ++i) { BasicBlock *TargetBlock = SI->getSuccessor(i); - ++SwitchEdges[TargetBlock]; updateReachableEdge(B, TargetBlock); } }