[PATCH] Fix unresolved linker symbols from misplaced extern definitions

[David Majnemer via llvm-commits]

On Wednesday, September 21, 2016, Chuck Atkins <chuck.atkins at kitware.com>
wrote:

> Hi David,
> Thanks for taking a look.  Yes, explicitly namespacing the externs
> fixes the issue in the same way. The failure is a link error with
> undefined references for LoopSimplifyID and LCSSAID in that object
> file.
>
> ../../lib/libLLVMTransformUtils.a(LoopUtils.cpp.o): In function
> `llvm::getLoopAnalysisUsage(llvm::AnalysisUsage&)':
> LoopUtils.cpp:(.text+0x33dd): undefined reference to `LoopSimplifyID'
> LoopUtils.cpp:(.text+0x3410): undefined reference to `LCSSAID'
>
>  Is there a reason to use a locally defined extern definition instead
> of pulling it in from the header it already resides in?


The comment block above the declaration explicitly says that it shouldn't
bring in the header.


>
> - Chuck
>
> On Tue, Sep 20, 2016 at 7:16 PM, David Majnemer
> <david.majnemer at gmail.com <javascript:;>> wrote:
> >
> >
> > On Tue, Sep 20, 2016 at 3:55 PM, David Majnemer <
> david.majnemer at gmail.com <javascript:;>>
> > wrote:
> >>
> >>
> >>
> >> On Fri, Sep 16, 2016 at 8:15 AM, Chuck Atkins via llvm-commits
> >> <llvm-commits at lists.llvm.org <javascript:;>> wrote:
> >>>
> >>> ping?
> >>>
> >>> Anybody mind taking a look at this patch?  It's fairly trivial; but
> >>> without it, builds with the Intel compiler fail.
> >>
> >>
> >> What is the failure mode?
> >
> >
> >
> > Out of curiosity, does the following make ICC any happier:
> >
> > diff --git a/lib/Transforms/Utils/LoopUtils.cpp
> > b/lib/Transforms/Utils/LoopUtils.cpp
> > index 8878689..1b2a2d6 100644
> > --- a/lib/Transforms/Utils/LoopUtils.cpp
> > +++ b/lib/Transforms/Utils/LoopUtils.cpp
> > @@ -929,6 +929,13 @@ SmallVector<Instruction *, 8>
> > llvm::findDefsUsedOutsideOfLoop(Loop *L) {
> >    return UsedOutside;
> >  }
> >
> > +// We locally access their IDs here because users shouldn't directly get
> > them
> > +// from this header.
> > +namespace llvm {
> > +extern char &LoopSimplifyID;
> > +extern char &LCSSAID;
> > +}
> > +
> >  void llvm::getLoopAnalysisUsage(AnalysisUsage &AU) {
> >    // By definition, all loop passes need the LoopInfo analysis and the
> >    // Dominator tree it depends on. Because they all participate in the
> loop
> > @@ -938,10 +945,7 @@ void llvm::getLoopAnalysisUsage(AnalysisUsage &AU)
> {
> >    AU.addRequired<LoopInfoWrapperPass>();
> >    AU.addPreserved<LoopInfoWrapperPass>();
> >
> > -  // We must also preserve LoopSimplify and LCSSA. We locally access
> their
> > IDs
> > -  // here because users shouldn't directly get them from this header.
> > -  extern char &LoopSimplifyID;
> > -  extern char &LCSSAID;
> > +  // We must also preserve LoopSimplify and LCSSA.
> >    AU.addRequiredID(LoopSimplifyID);
> >    AU.addPreservedID(LoopSimplifyID);
> >    AU.addRequiredID(LCSSAID);
> >
> >
> >>
> >>
> >>>
> >>>
> >>> On Tue, Sep 13, 2016 at 11:38 AM, Chuck Atkins <
> chuck.atkins at kitware.com <javascript:;>>
> >>> wrote:
> >>>>
> >>>> When LCSSAID and LoopSimplifyID are declared extern inside a function
> >>>> body,
> >>>> some linkers will fail to resolve the actual symbols defined in the
> llvm
> >>>> namespace.  By removing the function local extern declarations and
> >>>> relying
> >>>> on the header definitions in Scalar.h, the symbols can be properly
> >>>> reolved.
> >>>> This is specifically an issue when building with the Intel compiler.
> >>>> ---
> >>>>  lib/Transforms/Utils/LoopUtils.cpp | 3 +--
> >>>>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/lib/Transforms/Utils/LoopUtils.cpp
> >>>> b/lib/Transforms/Utils/LoopUtils.cpp
> >>>> index 3902c67..9a1cd0b 100644
> >>>> --- a/lib/Transforms/Utils/LoopUtils.cpp
> >>>> +++ b/lib/Transforms/Utils/LoopUtils.cpp
> >>>> @@ -26,6 +26,7 @@
> >>>>  #include "llvm/IR/ValueHandle.h"
> >>>>  #include "llvm/Pass.h"
> >>>>  #include "llvm/Support/Debug.h"
> >>>> +#include "llvm/Transforms/Scalar.h"
> >>>>  #include "llvm/Transforms/Utils/LoopUtils.h"
> >>>>
> >>>>  using namespace llvm;
> >>>> @@ -845,8 +846,6 @@ void llvm::getLoopAnalysisUsage(AnalysisUsage
> &AU) {
> >>>>
> >>>>    // We must also preserve LoopSimplify and LCSSA. We locally access
> >>>> their IDs
> >>>>    // here because users shouldn't directly get them from this header.
> >>>> -  extern char &LoopSimplifyID;
> >>>> -  extern char &LCSSAID;
> >>>>    AU.addRequiredID(LoopSimplifyID);
> >>>>    AU.addPreservedID(LoopSimplifyID);
> >>>>    AU.addRequiredID(LCSSAID);
> >>>> --
> >>>> 2.7.4
> >>>>
> >>>
> >>>
> >>> _______________________________________________
> >>> llvm-commits mailing list
> >>> llvm-commits at lists.llvm.org <javascript:;>
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >>>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160921/dae18cee/attachment.html>