[lld] r227784 - [ELF] Determine default search directories from Context.

Shankar Easwaran shankare at codeaurora.org
Sun Feb 1 22:31:03 PST 2015


On 2/2/2015 12:24 AM, Rui Ueyama wrote:
> On Sun, Feb 1, 2015 at 10:18 PM, Shankar Easwaran <shankare at codeaurora.org>
> wrote:
>
>> The reason was more that the Context should own what is the default search
>> directory. The ELFLinkingContext is using the baseTriple here not the real
>> triple.
>>
> Could you please elaborate why you think it should?
>
> My point was that this patch scattered the logic that used to be in one
> function and also added a new virtual public function that we used to not
> need. It doesn't seem like a good tradeoff.
Are you thinking that the default search path for all the ELF 
targets(aarch64, armv7, etc) be housed in the Driver ?  I would disagree.

>
>
>> IMO, Adding target specific search directory in the Gnu driver does not
>> make the code clean.
>>
>> Shankar Easwaran
>>
>>
>> On 2/2/2015 12:11 AM, Rui Ueyama wrote:
>>
>>> Does this actually improve the code quality? You added a new virtual
>>> function to the linking context, but the linking context are still using
>>> the triple to make a decision. This patch split a private function in the
>>> driver into two, and added them to the linking context as public member
>>> functions. Seems it's slightly worse than before.
>>>
>>> On Sun, Feb 1, 2015 at 10:00 PM, Shankar Easwaran <
>>> shankare at codeaurora.org>
>>> wrote:
>>>
>>>   Author: shankare
>>>> Date: Mon Feb  2 00:00:04 2015
>>>> New Revision: 227784
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=227784&view=rev
>>>> Log:
>>>> [ELF] Determine default search directories from Context.
>>>>
>>>> Target specific LinkingContext's  determine the default search directory.
>>>>
>>>> No change in functionality.
>>>>
>>>> Modified:
>>>>       lld/trunk/include/lld/Driver/Driver.h
>>>>       lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h
>>>>       lld/trunk/lib/Driver/GnuLdDriver.cpp
>>>>       lld/trunk/lib/ReaderWriter/ELF/X86/X86LinkingContext.h
>>>>
>>>> Modified: lld/trunk/include/lld/Driver/Driver.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/
>>>> Driver/Driver.h?rev=227784&r1=227783&r2=227784&view=diff
>>>>
>>>> ============================================================
>>>> ==================
>>>> --- lld/trunk/include/lld/Driver/Driver.h (original)
>>>> +++ lld/trunk/include/lld/Driver/Driver.h Mon Feb  2 00:00:04 2015
>>>> @@ -87,10 +87,6 @@ private:
>>>>      static bool applyEmulation(llvm::Triple &triple,
>>>>                                 llvm::opt::InputArgList &args,
>>>>                                 raw_ostream &diag);
>>>> -  static void addPlatformSearchDirs(ELFLinkingContext &ctx,
>>>> -                                    llvm::Triple &triple,
>>>> -                                    llvm::Triple &baseTriple);
>>>> -
>>>>      GnuLdDriver() LLVM_DELETED_FUNCTION;
>>>>    };
>>>>
>>>>
>>>> Modified: lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/ReaderWriter/
>>>> ELFLinkingContext.h?rev=227784&r1=227783&r2=227784&view=diff
>>>>
>>>> ============================================================
>>>> ==================
>>>> --- lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h (original)
>>>> +++ lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h Mon Feb  2
>>>> 00:00:04 2015
>>>> @@ -291,6 +291,11 @@ public:
>>>>      bool alignSegments() const { return _alignSegments; }
>>>>      void setAlignSegments(bool align) { _alignSegments = align; }
>>>>
>>>> +  /// \brief add platform specific search directories.
>>>> +  virtual void addDefaultSearchDirs(llvm::Triple & /*triple*/) {
>>>> +    addSearchPath("=/usr/lib");
>>>> +  }
>>>> +
>>>>    private:
>>>>      ELFLinkingContext() LLVM_DELETED_FUNCTION;
>>>>
>>>>
>>>> Modified: lld/trunk/lib/Driver/GnuLdDriver.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/
>>>> GnuLdDriver.cpp?rev=227784&r1=227783&r2=227784&view=diff
>>>>
>>>> ============================================================
>>>> ==================
>>>> --- lld/trunk/lib/Driver/GnuLdDriver.cpp (original)
>>>> +++ lld/trunk/lib/Driver/GnuLdDriver.cpp Mon Feb  2 00:00:04 2015
>>>> @@ -311,18 +311,6 @@ bool GnuLdDriver::applyEmulation(llvm::T
>>>>      return true;
>>>>    }
>>>>
>>>> -void GnuLdDriver::addPlatformSearchDirs(ELFLinkingContext &ctx,
>>>> -                                       llvm::Triple &triple,
>>>> -                                       llvm::Triple &baseTriple) {
>>>> -  if (triple.getOS() == llvm::Triple::NetBSD &&
>>>> -      triple.getArch() == llvm::Triple::x86 &&
>>>> -      baseTriple.getArch() == llvm::Triple::x86_64) {
>>>> -    ctx.addSearchPath("=/usr/lib/i386");
>>>> -    return;
>>>> -  }
>>>> -  ctx.addSearchPath("=/usr/lib");
>>>> -}
>>>> -
>>>>    #define LLVM_TARGET(targetName) \
>>>>      if ((p = elf::targetName##LinkingContext::create(triple))) return p;
>>>>
>>>> @@ -403,8 +391,9 @@ bool GnuLdDriver::parse(int argc, const
>>>>      for (auto libDir : parsedArgs->filtered(OPT_L))
>>>>        ctx->addSearchPath(libDir->getValue());
>>>>
>>>> +  // Add the default search directory specific to the target.
>>>>      if (!parsedArgs->hasArg(OPT_nostdlib))
>>>> -    addPlatformSearchDirs(*ctx, triple, baseTriple);
>>>> +    ctx->addDefaultSearchDirs(baseTriple);
>>>>
>>>>      // Handle --demangle option(For compatibility)
>>>>      if (parsedArgs->getLastArg(OPT_demangle))
>>>>
>>>> Modified: lld/trunk/lib/ReaderWriter/ELF/X86/X86LinkingContext.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/X86/
>>>> X86LinkingContext.h?rev=227784&r1=227783&r2=227784&view=diff
>>>>
>>>> ============================================================
>>>> ==================
>>>> --- lld/trunk/lib/ReaderWriter/ELF/X86/X86LinkingContext.h (original)
>>>> +++ lld/trunk/lib/ReaderWriter/ELF/X86/X86LinkingContext.h Mon Feb  2
>>>> 00:00:04 2015
>>>> @@ -36,6 +36,15 @@ public:
>>>>          return false;
>>>>        }
>>>>      }
>>>> +
>>>> +  void addDefaultSearchDirs(llvm::Triple &baseTriple) override {
>>>> +    if (_triple.getOS() == llvm::Triple::NetBSD &&
>>>> +        baseTriple.getArch() == llvm::Triple::x86_64) {
>>>> +      addSearchPath("=/usr/lib/i386");
>>>> +      return;
>>>> +    }
>>>> +    ELFLinkingContext::addDefaultSearchDirs(baseTriple);
>>>> +  }
>>>>    };
>>>>    } // end namespace elf
>>>>    } // end namespace lld
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
>> by the Linux Foundation
>>
>>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation




More information about the llvm-commits mailing list