The patch looks ok, thanks!<div>If it leads to performance problems, I agree we better fix them by playing with "branch weight metadata".</div><div>(I won't be able to check the performance until next week). </div>
<div><br></div><div>--kcc </div><div><br><div class="gmail_quote">On Tue, Jul 10, 2012 at 10:34 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This will in theory (hehe, see below) make many parts of LLVM work faster where the algorithms are tuned for "normal" CFG patterns in code. Previously ASan would create massive live ranges by inserting basic blocks onto the end of the function.<div>
<br></div><div>Currently this would cause some minor regressions in performance, but that will be fixed by a subsequent patch that attaches branch weight metadata to the appropriate places in the generated code. These will then allow the block placement pass to re-order the basic blocks into the optimal layout at the very end of codegen.</div>
<div><br></div><div>Sadly, this bright and shiny future is not where we are. Applying this patch and re-running the regression test Kostya added to PR13225 (which is actually a repro for PR12652) the codegen time slows down **massively**. The profile also looks quite different:</div>
<div><br></div><div> 29.03% llc llc [.] findLocalKill(unsigned int, llvm::MachineBasicBlock*, llvm::MachineInstr*, llvm::MachineRegisterInfo*, llvm::DenseMap<llvm::MachineInstr*, unsigned int, llvm::DenseMapInfo<llvm::MachineInstr*> >&) </div>
<div> 15.41% llc llc [.] void std::vector<llvm::MachineBasicBlock*, std::allocator<llvm::MachineBasicBlock*> >::_M_range_insert<std::reverse_iterator<__gnu_cxx::__normal_iterator<llvm::MachineBasicBlock**, std::vector<llvm::MachineBasicBlock*, std::allocator<llvm::MachineBasicBlock*> > > > >(__gnu_cxx::__normal_iterator<llvm::MachineBasicBlock**, std::vector<llvm::MachineBasicBlock*, std::allocator<llvm::MachineBasicBlock*> > >, std::reverse_iterator<__gnu_cxx::__normal_iterato</div>
<div> 8.05% llc llc [.] llvm::LiveVariables::MarkVirtRegAliveInBlock(llvm::LiveVariables::VarInfo&, llvm::MachineBasicBlock*, llvm::MachineBasicBlock*, std::vector<llvm::MachineBasicBlock*, std::allocator<llvm::MachineBasicBlock*> >&) </div>
<div> 7.80% llc llc [.] llvm::LiveInterval::addRangeFrom(llvm::LiveRange, llvm::LiveRange*) </div>
<div> 6.76% llc llc [.] llvm::LiveIntervals::handleVirtualRegisterDef(llvm::MachineBasicBlock*, llvm::MachineBasicBlock::bundle_iterator<llvm::MachineInstr, llvm::ilist_iterator<llvm::MachineInstr> >, llvm::SlotIndex, llvm::MachineOperand&, unsigned int, llvm::LiveInterval&) </div>
<div> 4.23% llc llc [.] (anonymous namespace)::TwoAddressInstructionPass::TryInstructionTransform(llvm::MachineBasicBlock::bundle_iterator<llvm::MachineInstr, llvm::ilist_iterator<llvm::MachineInstr> >&, llvm::MachineBasicBlock::bundle_iterator<llvm::MachineInstr, llvm::ilist_iterator<llvm::MachineInstr> >&, llvm::ilist_iterator<llvm::MachineBasicBlock>&, unsigned int, unsigned int, unsigned int, llvm::SmallPtrSet<llvm::MachineInstr*, 8u>&) </div>
<div> 3.30% llc llc [.] llvm::LiveInterval::extendIntervalEndTo(llvm::LiveRange*, llvm::SlotIndex) </div>
<div> 2.89% llc llc [.] llvm::SparseBitVector<128u>::test(unsigned int) </div>
<div> 2.68% llc llc [.] llvm::SparseBitVector<128u>::FindLowerBound(unsigned int) </div>
<div> 2.17% llc llc [.] llvm::SparseBitVector<128u>::set(unsigned int) </div>
<div> 0.80% llc llc [.] llvm::LiveVariables::HandleVirtRegUse(unsigned int, llvm::MachineBasicBlock*, llvm::MachineInstr*) </div>
<div> 0.65% llc <a href="http://libc-2.13.so" target="_blank">libc-2.13.so</a> [.] _int_malloc </div>
<div> 0.46% llc llc [.] llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*, unsigned char const*, unsigned int) </div>
<div> 0.42% llc llc [.] llvm::SparseBitVector<128u>::SparseBitVectorIterator::AdvanceToNextNonZero() </div>
<div> 0.38% llc llc [.] llvm::SmallPtrSetImpl::insert_imp(void const*) </div>
<div> 0.37% llc llc [.] (anonymous namespace)::DAGCombiner::visit(llvm::SDNode*) </div>
<div> 0.37% llc llc [.] llvm::SmallVectorImpl<llvm::LiveRange>::insert(llvm::LiveRange*, llvm::LiveRange const&)</div><div><br></div><div><br></div><div>So, while I think this is definitely the right way to go eventually (and if you can go ahead and review, that would be appreciated) clearly we can't commit this until this is a bit less severe... ;]</div>
<div><br></div><div>I think making 'join' use my new merge logic may help some of these (the addRangeFrom in particular), others look largely unrelated.</div><span class="HOEnZb"><font color="#888888"><div><br></div>
<div>-Chandler</div>
</font></span></blockquote></div><br></div>