Don't count debug instructions when limit the number of checked instructions. Otherwise the debug information may impact optimization like the test case shows.
Details
Details
Diff Detail
Diff Detail
Event Timeline
| llvm/test/Transforms/GVN/PRE/pre-load-dbg.ll | ||
|---|---|---|
| 2 | Pass -gvn-max-num-insns with a low value and reduce test accordingly. No need to test the actual 100 instruction limit. | |
Comment Actions
If I understand this limitation correctly, its only goal is to save compile time. What are you trying to achieve? Good performance in debug mode or?..
Comment Actions
There are other limits like that where we dont' care about it:
// Find non-clobbered value for Loc memory location in extended basic block
// (chain of basic blocks with single predecessors) starting From instruction.
static Value *findDominatingValue(const MemoryLocation &Loc, Type *LoadTy,
Instruction *From, AAResults *AA) {
uint32_t NumVisitedInsts = 0;
BasicBlock *FromBB = From->getParent();
BatchAAResults BatchAA(*AA);
for (BasicBlock *BB = FromBB; BB; BB = BB->getSinglePredecessor())
for (auto I = BB == FromBB ? From->getReverseIterator() : BB->rbegin(),
E = BB->rend();
I != E; ++I) {
// Stop the search if limit is reached.
if (++NumVisitedInsts > MaxNumVisitedInsts)
return nullptr;
Instruction *Inst = &*I;
if (isModSet(BatchAA.getModRefInfo(Inst, Loc)))
return nullptr;
if (auto *LI = dyn_cast<LoadInst>(Inst))
if (LI->getPointerOperand() == Loc.Ptr && LI->getType() == LoadTy)
return LI;
}
return nullptr;
}Why getting GVN to work here despite giant CT cost is important?
Comment Actions
From a debug-info perspective this LGTM, with the test improvement to reduce the size -- I'm not familiar with all of GVN though. The objective of ensuring that debug-info can't affect the code generated is an important one.
You might consider using the BasicBlock::instructionsWithoutDebug range to iterate over instructions without any explicit logic to deal with debug-info and pseudo instructions.
Comment Actions
@mkazantsev I believe it's just the general policy that debuginfo should not affect optimization outcome. We do generally exclude debug intrinsics for these kinds of instruction counting cutoffs for that reason.
Comment Actions
Well, if so, then LG with Nikita's comment regarding the test. It needs to be reasonably small for a change like this.
Can we follow-up updating the place I've pointed out above?
Pass -gvn-max-num-insns with a low value and reduce test accordingly. No need to test the actual 100 instruction limit.