Ok, I think this patch is viable to go into the tree. The timings aren't great, but they're not substantially worse than the old block ordering now, and in some cases they're better:<div><br></div><div><div>% for ((i=$((2 ** 10)); i <= $((32 * 2 ** 10)); i*=2)); do ./asan_glob.sh $i > x2.c; echo $i; time ./bin/clang -faddress-sanitizer -mllvm -asan-max-ins-per-bb=99999999 -O3 -o /dev/null -c x2.c; done</div>
<div>1024</div><div>./bin/clang -faddress-sanitizer -mllvm -asan-max-ins-per-bb=99999999 -O3 -o 1.11s user 0.03s system 99% cpu 1.139 total</div><div>2048</div><div>./bin/clang -faddress-sanitizer -mllvm -asan-max-ins-per-bb=99999999 -O3 -o 2.05s user 0.04s system 99% cpu 2.090 total</div>
<div>4096</div><div>./bin/clang -faddress-sanitizer -mllvm -asan-max-ins-per-bb=99999999 -O3 -o 6.40s user 0.10s system 99% cpu 6.512 total</div><div>8192</div><div>./bin/clang -faddress-sanitizer -mllvm -asan-max-ins-per-bb=99999999 -O3 -o 27.39s user 0.37s system 99% cpu 27.815 total</div>
<div>16384</div><div>./bin/clang -faddress-sanitizer -mllvm -asan-max-ins-per-bb=99999999 -O3 -o 121.61s user 1.50s system 99% cpu 2:03.34 total</div><div>32768</div><div>./bin/clang -faddress-sanitizer -mllvm -asan-max-ins-per-bb=99999999 -O3 -o 794.23s user 15.56s system 93% cpu 14:25.32 total</div>
</div><div><br></div><div>It's still growing faster than I would like, but frankly, with 32k stores to a global array in a single function, there really isn't much more we can do here.</div><div><br></div><div>I'm working on the patch to add branch weights, but it's incredibly simple, so I'm not worried about checking this one in in the interim.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jul 11, 2012 at 10:26 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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 class="HOEnZb"><div class="h5"><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><font color="#888888"><div><br></div>
<div>-Chandler</div>
</font></span></blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>