[PATCH] Fix unresolved linker symbols from misplaced extern definitions

[Chuck Atkins via llvm-commits]

I guess I misunderstood the comment.  My fault.  I'll re-create the
patch with the namespaced externs instead of pulling in the header.
----------
Chuck Atkins
Staff R&D Engineer, Scientific Computing
Kitware, Inc.
(518) 881-1183


On Wed, Sep 21, 2016 at 10:15 AM, David Majnemer
<david.majnemer at gmail.com> wrote:
>
>
> 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> wrote:
>> >
>> >
>> > On Tue, Sep 20, 2016 at 3:55 PM, David Majnemer
>> > <david.majnemer at gmail.com>
>> > wrote:
>> >>
>> >>
>> >>
>> >> On Fri, Sep 16, 2016 at 8:15 AM, Chuck Atkins via llvm-commits
>> >> <llvm-commits at lists.llvm.org> 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>
>> >>> 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
>> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> >>>
>> >>
>> >