[llvm-commits] PATCH: Fix AddressSanitizer to emit basic blocks in the natural order for the CFG

Kostya Serebryany kcc at google.com
Wed Jul 11 10:26:46 PDT 2012


The patch looks ok, thanks!
If it leads to performance problems, I agree we better fix them by playing
with "branch weight metadata".
(I won't be able to check the performance until next week).

--kcc

On Tue, Jul 10, 2012 at 10:34 AM, Chandler Carruth <chandlerc at gmail.com>wrote:

> 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.
>
> 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.
>
> 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:
>
>     29.03%      llc  llc                  [.] findLocalKill(unsigned int,
> llvm::MachineBasicBlock*, llvm::MachineInstr*, llvm::MachineRegisterInfo*,
> llvm::DenseMap<llvm::MachineInstr*, unsigned int,
> llvm::DenseMapInfo<llvm::MachineInstr*> >&)
>
>
>
>     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
>      8.05%      llc  llc                  [.]
> llvm::LiveVariables::MarkVirtRegAliveInBlock(llvm::LiveVariables::VarInfo&,
> llvm::MachineBasicBlock*, llvm::MachineBasicBlock*,
> std::vector<llvm::MachineBasicBlock*,
> std::allocator<llvm::MachineBasicBlock*> >&)
>
>
>
>      7.80%      llc  llc                  [.]
> llvm::LiveInterval::addRangeFrom(llvm::LiveRange, llvm::LiveRange*)
>
>
>
>
>
>
>      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&)
>
>
>
>      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>&)
>
>      3.30%      llc  llc                  [.]
> llvm::LiveInterval::extendIntervalEndTo(llvm::LiveRange*, llvm::SlotIndex)
>
>
>
>
>
>
>      2.89%      llc  llc                  [.]
> llvm::SparseBitVector<128u>::test(unsigned int)
>
>
>
>
>
>
>      2.68%      llc  llc                  [.]
> llvm::SparseBitVector<128u>::FindLowerBound(unsigned int)
>
>
>
>
>
>
>      2.17%      llc  llc                  [.]
> llvm::SparseBitVector<128u>::set(unsigned int)
>
>
>
>
>
>
>      0.80%      llc  llc                  [.]
> llvm::LiveVariables::HandleVirtRegUse(unsigned int,
> llvm::MachineBasicBlock*, llvm::MachineInstr*)
>
>
>
>
>
>      0.65%      llc  libc-2.13.so         [.] _int_malloc
>
>
>
>
>
>
>      0.46%      llc  llc                  [.]
> llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*, unsigned char
> const*, unsigned int)
>
>
>
>
>
>      0.42%      llc  llc                  [.]
> llvm::SparseBitVector<128u>::SparseBitVectorIterator::AdvanceToNextNonZero()
>
>
>
>
>
>
>      0.38%      llc  llc                  [.]
> llvm::SmallPtrSetImpl::insert_imp(void const*)
>
>
>
>
>
>
>      0.37%      llc  llc                  [.] (anonymous
> namespace)::DAGCombiner::visit(llvm::SDNode*)
>
>
>
>
>
>      0.37%      llc  llc                  [.]
> llvm::SmallVectorImpl<llvm::LiveRange>::insert(llvm::LiveRange*,
> llvm::LiveRange const&)
>
>
> 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... ;]
>
> I think making 'join' use my new merge logic may help some of these (the
> addRangeFrom in particular), others look largely unrelated.
>
> -Chandler
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120711/eac8ba56/attachment.html>


More information about the llvm-commits mailing list