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

[Rui Ueyama]

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.


> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150201/9208572d/attachment.html>