Skip to content

Commit

Permalink
fix: rearrange so assert(false) not on return path
Browse files Browse the repository at this point in the history
  • Loading branch information
priyasiddharth committed Nov 2, 2022
1 parent 156e5f1 commit 2687495
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 27 deletions.
11 changes: 6 additions & 5 deletions lib/seadsa/Graph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ void Node::addAccessedType(unsigned off, llvm::Type *type) {
tmp.push_back({o + i * sz, elementTy});

workList.insert(workList.end(), tmp.rbegin(), tmp.rend());
} else if (const FixedVectorType *vty = dyn_cast<const FixedVectorType>(t)) {
} else if (const FixedVectorType *vty =
dyn_cast<const FixedVectorType>(t)) {
uint64_t sz = vty->getElementType()->getPrimitiveSizeInBits() / 8;
WorkList tmp;
const size_t numElements(vty->getNumElements());
Expand Down Expand Up @@ -398,9 +399,8 @@ const Cell &Node::getLink(Field _f) const {
assert(!FieldType::IsNotTypeAware());
assert(!f.hasOmniType());
Field omni = f.mkOmniField();
if (m_links.count(omni)) return *m_links.at(omni);

assert(false); // unreachable
assert(m_links.count(omni));
return *m_links.at(omni);
}

void Node::setLink(const Field _f, const Cell &c) {
Expand Down Expand Up @@ -463,7 +463,8 @@ void Node::addLink(Field _f, const Cell &c) {
m_links = std::move(new_links);

// -- recreate links that have been removed by adding them through a cell
// -- pointing to the current node. The cell takes care of resolving forwarding
// -- pointing to the current node. The cell takes care of resolving
// forwarding
Cell cc(*this, 0);
for (auto &kv : saved_links) {
cc.addLink(kv.first, *kv.second);
Expand Down
43 changes: 21 additions & 22 deletions lib/seadsa/RemovePtrToInt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,14 @@ static bool visitStoreInst(StoreInst *SI, Function &F, const DataLayout &DL,
class PointerPromoter : public InstVisitor<PointerPromoter, Value *> {
Type *m_ty;
SmallPtrSetImpl<Instruction *> &m_toRemove;
DenseMap<Value*, Value*> &m_toReplace;
DenseMap<Value *, Value *> &m_toReplace;
// -- to break cycles in PHINodes
SmallPtrSet<Instruction *, 16> m_visited;

public:
PointerPromoter(SmallPtrSetImpl<Instruction *> &toRemove, DenseMap<Value*, Value*> &toReplace)
: m_ty(nullptr), m_toRemove(toRemove), m_toReplace(toReplace) {}
PointerPromoter(SmallPtrSetImpl<Instruction *> &toRemove,
DenseMap<Value *, Value *> &toReplace)
: m_ty(nullptr), m_toRemove(toRemove), m_toReplace(toReplace) {}

Value *visitInstruction(Instruction &I) { return nullptr; }

Expand Down Expand Up @@ -227,8 +228,9 @@ class PointerPromoter : public InstVisitor<PointerPromoter, Value *> {
IRBuilder<> IRB(&I);

ptr = IRB.CreateBitCast(ptr, IRB.getInt8PtrTy());
auto *gep = IRB.CreateGEP(ptr->getType()->getScalarType()->getPointerElementType(),
ptr, {I.getOperand(1)});
auto *gep =
IRB.CreateGEP(ptr->getType()->getScalarType()->getPointerElementType(),
ptr, I.getOperand(1));
return IRB.CreateBitCast(gep, m_ty);
}

Expand Down Expand Up @@ -270,7 +272,7 @@ class PointerPromoter : public InstVisitor<PointerPromoter, Value *> {
m_toRemove.insert(SI);
} else if (isa<IntToPtrInst>(u)) {
/* skip */;
} else {
} else {
// -- if there is an interesting user, schedule it to be replaced
if (!p2i) {
IRB.SetInsertPoint(&I);
Expand Down Expand Up @@ -303,12 +305,11 @@ class PointerPromoter : public InstVisitor<PointerPromoter, Value *> {
}
};

static bool
visitIntToPtrInst(IntToPtrInst *I2P, Function &F, const DataLayout &DL,
DominatorTree &DT,
SmallDenseMap<PHINode *, PHINode *> &NewPhis,
SmallPtrSetImpl<Instruction *> &MaybeUnusedInsts,
DenseMap<Value*, Value*> &RenameMap) {
static bool visitIntToPtrInst(IntToPtrInst *I2P, Function &F,
const DataLayout &DL, DominatorTree &DT,
SmallDenseMap<PHINode *, PHINode *> &NewPhis,
SmallPtrSetImpl<Instruction *> &MaybeUnusedInsts,
DenseMap<Value *, Value *> &RenameMap) {
assert(I2P);

auto *IntVal = I2P->getOperand(0);
Expand Down Expand Up @@ -413,7 +414,7 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
SmallPtrSet<StoreInst *, 8> StoresToErase;
SmallPtrSet<Instruction *, 16> MaybeUnusedInsts;
DenseMap<Value*, Value*> RenameMap;
DenseMap<Value *, Value *> RenameMap;
SmallDenseMap<PHINode *, PHINode *> NewPhis;

for (auto &BB : F) {
Expand All @@ -429,7 +430,8 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
}

if (auto *I2P = dyn_cast<IntToPtrInst>(&I)) {
Changed |= visitIntToPtrInst(I2P, F, DL, DT, NewPhis, MaybeUnusedInsts, RenameMap);
Changed |= visitIntToPtrInst(I2P, F, DL, DT, NewPhis, MaybeUnusedInsts,
RenameMap);
continue;
}
}
Expand All @@ -442,7 +444,7 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
llvm::make_filter_range(MaybeUnusedInsts, [](Instruction *I) {
return isInstructionTriviallyDead(I);
}));

// TODO: RecursivelyDeleteTriviallyDeadInstructions takes a vector of
// WeakTrackingVH since LLVM 11, which implies that `MaybeUnusedInsts` should
// be a vector of WeakTrackingVH as well. See comment:
Expand All @@ -451,11 +453,10 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
// removed before this call. So it should be safe to keep its original type
// and map it to WeakTrackingVH as the following code does.
SmallVector<WeakTrackingVH, 16> TriviallyDeadHandles;
for (auto i : TriviallyDeadInstructions)
TriviallyDeadHandles.emplace_back(WeakTrackingVH(i));

RecursivelyDeleteTriviallyDeadInstructions(TriviallyDeadHandles);
for (auto i : TriviallyDeadInstructions)
TriviallyDeadHandles.emplace_back(WeakTrackingVH(i));

RecursivelyDeleteTriviallyDeadInstructions(TriviallyDeadHandles);

for (auto kv : RenameMap) {
kv.first->replaceAllUsesWith(kv.second);
Expand All @@ -464,9 +465,7 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
DOG(llvm::errs() << "\n~~~~~~~ End of RP2I on " << F.getName() << " ~~~~~ \n";
llvm::errs().flush());

if (Changed) {
assert(!llvm::verifyFunction(F, &llvm::errs()));
}
if (Changed) { assert(!llvm::verifyFunction(F, &llvm::errs())); }

// llvm::errs() << F << "\n";
return Changed;
Expand Down

0 comments on commit 2687495

Please sign in to comment.