[lld] r222983 - [Core] Add flag to check if RoundTripPasses need to be run.

[Rui Ueyama]

On Mon, Dec 1, 2014 at 1:48 PM, Nick Kledzik <kledzik at apple.com> wrote:

>
> On Dec 1, 2014, at 12:34 PM, Rui Ueyama <ruiu at google.com> wrote:
>
> That sounds like we really need a new property of Atom.
>
> 1. If we run LLD in Release build, the roundtrip passes don't run, so
> everything works fine.
> 2. If we run LLD in Debug build (and from the unit tests), the information
> is dropped during the round-trip conversion, and it fails.
> 3. RoundTrip tests should't drop any information.
>
> 2 and 3 conflicts.
>
>
> I have a different view.
>
> I think of those Passes as a quick way to exercise the YAML and native
> formats.  The mechanism is as if you went through the test cases and added
> a line which ran “ld -r” to merge all input files into one file, then ran
> the rest of the test with that merged input file.  That works everywhere
> except in tests that care about the name of the input file (which is now
> changed).  So, for those cases you would not do the merge step.  But since
> the round trip is implicit, there is no way to turn it off.  Shankar is
> adding a way to turn it off.
>

That doesn't sounds like a different view.

If a test fails because of the difference of an input file name, not
because the round-trip conversion drops some information, we should disable
the round-trip test only for that test. Now I feel I understood your
suggestion in a different thread. Is there any way to override the
environment variable in the lit test? Does it allow us to write RUN:
LLD_ROUNDTRIP_TEST=0 lld <args here> ?


>
> I should note that, again, I don't actually like the idea of YAML/Native
> format, though. It feels like it doesn't worth the cost of maintaining two
> more different outputs.
>
>
> Those formats exist for different reasons:
>
> YAML exists because it is the preferred way to write core test cases.
>
> “Native” exists because eventually we want a way to improve linker
> performance by reducing the parsing overhead in the front end of the
> linker. During development, the compiler writes each .o file once, but the
> linker reads it many times (e.g. each time you one file and run ‘make’, the
> linker has to re-parse every .o file again).  So the ideal .o format is one
> that is fast to parse (not one that is fast to write.  The native format
> "keeps us honest” that the atom/reference model can be flattened to disk.
>

I understand that point, and it would be nice if we have object file
formats that's faster to parse. But currently no one produces the object
file in such format except LLD and that doesn't seem to change soon. We are
currently maintaining the stuff for nothing. I don't oppose to invest in
new format but I wanted to be convinced that the format would actually be
used in the future.


> -Nick
>
>
>
> On Mon, Dec 1, 2014 at 12:20 PM, Shankar Easwaran <shankare at codeaurora.org
> > wrote:
>
>> Hi Rui,
>>
>> We discussed to add a new property to the atom, but its really not needed
>> as the original filename from where the atom was parsed is available in
>> release mode(roundtrip passes dont get called in release mode).
>>
>> The discussion was here, http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-
>> November/078910.html.
>>
>> Shankar Easwaran
>>
>>
>> On 12/1/2014 2:14 PM, Rui Ueyama wrote:
>>
>>> On Mon, Dec 1, 2014 at 12:11 PM, Shankar Easwaran <
>>> shankare at codeaurora.org>
>>> wrote:
>>>
>>>  On 12/1/2014 2:04 PM, Rui Ueyama wrote:
>>>>
>>>>  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?
>>>>>
>>>>>  Yes, I thought of adding a way to bypass that but you are right, we
>>>> should
>>>> not allow the RoundTripPasses to be bypassed. I will remove the
>>>> setRunRoundTripPass function in LinkingContext.
>>>>
>>>> But I do need lld to preserve the filename of the atom using references
>>>> if
>>>> the round trip passes were going to be run. So the reader and the writer
>>>> would read the information using references if the round trip passes
>>>> were
>>>> being run.
>>>>
>>>
>>> If you need a new property, you can add that to Atom. (Code review would
>>> be
>>> needed though.)
>>>
>>>
>>>  Shankar Easwaran
>>>>
>>>>
>>>> --
>>>> 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141201/77793e0d/attachment.html>