[lld] r210008 - Simplify markLive().

Rui Ueyama ruiu at google.com
Mon Jun 2 17:57:00 PDT 2014


On Mon, Jun 2, 2014 at 2:02 PM, Nick Kledzik <kledzik at apple.com> wrote:

>
> On Jun 2, 2014, at 1:06 AM, Rui Ueyama <ruiu at google.com> wrote:
> > Author: ruiu
> > Date: Mon Jun  2 03:06:57 2014
> > New Revision: 210008
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=210008&view=rev
> > Log:
> > Simplify markLive().
> >
> > Reference::target() never returns a nullptr, so NULL check
> > is not needed and is more harmful than doing nothing.
> > No functionality change.
> Reference::target() can return nullptr.  From the header:
>
>   /// If the reference is an edge to another Atom, then this returns the
>   /// other Atom.  Otherwise, it returns nullptr.
>   virtual const Atom *target() const = 0;
>
> As the comment for the Reference class explains, they can be used for
> carrying
> extra data about an atom in which case the target() part is unused.


Is that comment still relevant? There are other places that assumes
target() never returns nullptr, and all uses of References in LLD are to
represent edges in graph of Atoms. The name "reference" also implies that
it refers something.

So, if we need an attribute that has no target, I feel like it should be
represented by something other than Reference.


> -Nick
>
>
> >
> > Modified:
> >    lld/trunk/include/lld/Core/Resolver.h
> >    lld/trunk/lib/Core/Resolver.cpp
> >
> > Modified: lld/trunk/include/lld/Core/Resolver.h
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Core/Resolver.h?rev=210008&r1=210007&r2=210008&view=diff
> >
> ==============================================================================
> > --- lld/trunk/include/lld/Core/Resolver.h (original)
> > +++ lld/trunk/include/lld/Core/Resolver.h Mon Jun  2 03:06:57 2014
> > @@ -67,7 +67,7 @@ private:
> >   void checkDylibSymbolCollisions();
> >   void forEachUndefines(bool searchForOverrides, UndefCallback callback);
> >
> > -  void markLive(const Atom &atom);
> > +  void markLive(const Atom *atom);
> >   void addAtoms(const std::vector<const DefinedAtom *>&);
> >
> >   class MergedFile : public MutableFile {
> >
> > Modified: lld/trunk/lib/Core/Resolver.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/Resolver.cpp?rev=210008&r1=210007&r2=210008&view=diff
> >
> ==============================================================================
> > --- lld/trunk/lib/Core/Resolver.cpp (original)
> > +++ lld/trunk/lib/Core/Resolver.cpp Mon Jun  2 03:06:57 2014
> > @@ -305,17 +305,16 @@ void Resolver::updateReferences() {
> > }
> >
> > // For dead code stripping, recursively mark atoms "live"
> > -void Resolver::markLive(const Atom &atom) {
> > +void Resolver::markLive(const Atom *atom) {
> >   // Mark the atom is live. If it's already marked live, then stop
> recursion.
> > -  auto exists = _liveAtoms.insert(&atom);
> > +  auto exists = _liveAtoms.insert(atom);
> >   if (!exists.second)
> >     return;
> >
> >   // Mark all atoms it references as live
> > -  if (const DefinedAtom *defAtom = dyn_cast<DefinedAtom>(&atom))
> > +  if (const DefinedAtom *defAtom = dyn_cast<DefinedAtom>(atom))
> >     for (const Reference *ref : *defAtom)
> > -      if (const Atom *target = ref->target())
> > -        markLive(*target);
> > +      markLive(ref->target());
> > }
> >
> > // remove all atoms not actually used
> > @@ -342,7 +341,7 @@ void Resolver::deadStripOptimize() {
> >
> >   // mark all roots as live, and recursively all atoms they reference
> >   for (const Atom *dsrAtom : _deadStripRoots)
> > -    markLive(*dsrAtom);
> > +    markLive(dsrAtom);
> >
> >   // now remove all non-live atoms from _atoms
> >   _atoms.erase(std::remove_if(_atoms.begin(), _atoms.end(), [&](const
> Atom *a) {
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140602/6cac3aa0/attachment.html>


More information about the llvm-commits mailing list