[lld] r227784 - [ELF] Determine default search directories from Context.
Michael Spencer
bigcheesegs at gmail.com
Tue Feb 3 17:33:04 PST 2015
On Sun, Feb 1, 2015 at 10:31 PM, Shankar Easwaran
<shankare at codeaurora.org> wrote:
> 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.
They should. It's the drivers job to interact with the environment to
figure out how to drive the linker. This is also how it works in
clang.
- Michael Spencer
>
>
>>
>>
>>> 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
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list