[lld] r222983 - [Core] Add flag to check if RoundTripPasses need to be run.
Rui Ueyama
ruiu at google.com
Mon Dec 1 12:04:26 PST 2014
I'm not sure if I got what you mean... So, just to make sure we are on the
same page, the reason to have the round-trip passes is to make sure all
output can be serialized and de-serialized, right? I think you added these
tests because of that reason.
Because the YAML/Native are guaranteed compatible, we can dump an
intermediate results to YAML/Native files and then resume processing by
reading them back.
And then looks like you are now adding a way to bypass that. So they are no
longer compatible. That is exactly what you wanted to avoid, no?
On Mon, Dec 1, 2014 at 11:34 AM, Shankar Easwaran <shankare at codeaurora.org>
wrote:
> Thanks for your review, Rui.
>
> I was thinking that it would be nice to have a no-round trip pass, if we
> dont want to spend time in roundtrip passes during DEBUG mode.
>
> I will remove the set function in the LinkingContext since there is no use
> for the setRoundTripPass outside LinkingContext.
>
> I will keep the flag though in the linking context, so that the ELF
> reader and the ELFWriter can use that information to find out if the
> RoundTrip passes were run or not.
>
> Shankar Easwaran
>
>
> On 12/1/2014 1:11 PM, Rui Ueyama wrote:
>
>> I don't actually like these tests and the very idea of supporting object
>> file serialization to a YAML and "Native" format, but they are there and
>> it's supposed to be able to serialize and de-serialize all information.
>> These tests are to ensure the capability by dumping and reloading a file
>> two times, in YAML and Native format, and verifying nothing was broken.
>>
>> Any files that lld is able to handle should pass the round-trip test. Even
>> if it's a platform-specific one, because every object file is in some
>> degree platform-dependent.
>>
>> So, in the above sense, this patch doesn't look good to me. It provides a
>> way to circumvent the tests. If we enable this, YAML and Native format
>> support will silently regress over time.
>>
>> If we take this way, I'd suggest removing YAML and Native support.
>>
>> If not, we should add the property you need to File and properly save and
>> restore from YAML/Native files.
>>
>> On Sun, Nov 30, 2014 at 5:04 PM, Shankar Easwaran <
>> shankare at codeaurora.org>
>> wrote:
>>
>> Author: shankare
>>> Date: Sun Nov 30 19:04:11 2014
>>> New Revision: 222983
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=222983&view=rev
>>> Log:
>>> [Core] Add flag to check if RoundTripPasses need to be run.
>>>
>>> This would allow other flavor specific contexts to override the default
>>> value,
>>> if they want to optionally run the round trip passes.
>>>
>>> There is some information lost like the original file owner of the atom
>>> with
>>> RoundTripPasses. The Gnu flavor needs this information inorder to
>>> implement
>>> LinkerScript matching and for other diagnostic outputs such as Map files.
>>>
>>> The flag also can be used to record information in the Atom if the
>>> information
>>> to the Writer needs to be conveyed through References too.
>>>
>>> Modified:
>>> lld/trunk/include/lld/Core/LinkingContext.h
>>> lld/trunk/lib/Core/LinkingContext.cpp
>>> lld/trunk/lib/Driver/Driver.cpp
>>>
>>> Modified: lld/trunk/include/lld/Core/LinkingContext.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/
>>> Core/LinkingContext.h?rev=222983&r1=222982&r2=222983&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/include/lld/Core/LinkingContext.h (original)
>>> +++ lld/trunk/include/lld/Core/LinkingContext.h Sun Nov 30 19:04:11 2014
>>> @@ -318,8 +318,15 @@ public:
>>> /// Return the next ordinal and Increment it.
>>> virtual uint64_t getNextOrdinalAndIncrement() const { return
>>> _nextOrdinal++; }
>>>
>>> - /// @}
>>> +#ifndef NDEBUG
>>> + void setRunRoundTripPass(bool roundTripPass) {
>>> + _runRoundTripPasses = roundTripPass;
>>> + }
>>> +
>>> + bool runRoundTripPass() const { return _runRoundTripPasses; }
>>> +#endif
>>>
>>> + /// @}
>>> protected:
>>> LinkingContext(); // Must be subclassed
>>>
>>> @@ -350,6 +357,9 @@ protected:
>>> bool _allowRemainingUndefines;
>>> bool _logInputFiles;
>>> bool _allowShlibUndefines;
>>> +#ifndef NDEBUG
>>> + bool _runRoundTripPasses;
>>> +#endif
>>> OutputFileType _outputFileType;
>>> std::vector<StringRef> _deadStripRoots;
>>> std::map<std::string, std::string> _aliasSymbols;
>>>
>>> Modified: lld/trunk/lib/Core/LinkingContext.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Core/
>>> LinkingContext.cpp?rev=222983&r1=222982&r2=222983&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/lib/Core/LinkingContext.cpp (original)
>>> +++ lld/trunk/lib/Core/LinkingContext.cpp Sun Nov 30 19:04:11 2014
>>> @@ -13,9 +13,28 @@
>>> #include "lld/Core/Simple.h"
>>> #include "lld/ReaderWriter/Writer.h"
>>> #include "llvm/ADT/Triple.h"
>>> +#include "llvm/Support/Process.h"
>>>
>>> namespace lld {
>>>
>>> +#ifndef NDEBUG
>>> +LinkingContext::LinkingContext()
>>> + : _deadStrip(false), _allowDuplicates(false),
>>> + _globalsAreDeadStripRoots(false),
>>> + _searchArchivesToOverrideTentativeDefinitions(false),
>>> + _searchSharedLibrariesToOverrideTentativeDefinitions(false),
>>> + _warnIfCoalesableAtomsHaveDifferentCanBeNull(false),
>>> + _warnIfCoalesableAtomsHaveDifferentLoadName(false),
>>> + _printRemainingUndefines(true), _allowRemainingUndefines(false),
>>> + _logInputFiles(false), _allowShlibUndefines(false),
>>> + _runRoundTripPasses(false),
>>> _outputFileType(OutputFileType::Default),
>>> + _nextOrdinal(0) {
>>> + llvm::Optional<std::string> env =
>>> + llvm::sys::Process::GetEnv("LLD_RUN_ROUNDTRIP_TEST");
>>> + if (env.hasValue() && !env.getValue().empty())
>>> + setRunRoundTripPass(true);
>>> +}
>>> +#else
>>> LinkingContext::LinkingContext()
>>> : _deadStrip(false), _allowDuplicates(false),
>>> _globalsAreDeadStripRoots(false),
>>> @@ -26,6 +45,7 @@ LinkingContext::LinkingContext()
>>> _printRemainingUndefines(true), _allowRemainingUndefines(false),
>>> _logInputFiles(false), _allowShlibUndefines(false),
>>> _outputFileType(OutputFileType::Default), _nextOrdinal(0) {}
>>> +#endif
>>>
>>> LinkingContext::~LinkingContext() {}
>>>
>>>
>>> Modified: lld/trunk/lib/Driver/Driver.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/
>>> Driver.cpp?rev=222983&r1=222982&r2=222983&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/lib/Driver/Driver.cpp (original)
>>> +++ lld/trunk/lib/Driver/Driver.cpp Sun Nov 30 19:04:11 2014
>>> @@ -23,7 +23,6 @@
>>> #include "llvm/Support/CommandLine.h"
>>> #include "llvm/Support/FileSystem.h"
>>> #include "llvm/Support/Path.h"
>>> -#include "llvm/Support/Process.h"
>>> #include "llvm/Support/raw_ostream.h"
>>> #include <mutex>
>>>
>>> @@ -114,8 +113,7 @@ bool Driver::link(LinkingContext &contex
>>> context.addPasses(pm);
>>>
>>> #ifndef NDEBUG
>>> - llvm::Optional<std::string> env =
>>> llvm::sys::Process::GetEnv("LLD_RUN_ROUNDTRIP_TEST");
>>> - if (env.hasValue() && !env.getValue().empty()) {
>>> + if (context.runRoundTripPass()) {
>>> pm.add(std::unique_ptr<Pass>(new RoundTripYAMLPass(context)));
>>> pm.add(std::unique_ptr<Pass>(new RoundTripNativePass(context)));
>>> }
>>>
>>>
>>> _______________________________________________
>>> 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/20141201/37be6606/attachment.html>
More information about the llvm-commits
mailing list