[PATCH] [PECOFF][ELF][Darwin] Changes to fix some real issues in code

[Rui Ueyama]

On Mon, Oct 28, 2013 at 11:55 AM, Shankar Easwaran
<shankare at codeaurora.org>wrote:

>
> On 10/28/2013 1:26 PM, Rui Ueyama wrote:
>
>> Ah, there was a misunderstanding there. What I meant was that patch file
>> attachments, as opposed to setting up a new git repository, is of course
>> OK.
>>
> Ah.
>
>  r193478 and r193477 are LGTM. They would fix the tests failing with
>> RoundTrip tests. r193479 is however wouldn't.
>>
> These changes did fix some more of the failures.
>
> For example I couldnt figure out where the ImportDirectoryAtom gets added
> to the PassManager file.
>
> For example, the lld Pass calls createImportDirectory, which adds to the
> list of Import Directories.
>
> I looked at the YAML files too before adding those changes, and they didnt
> appear in the YAML file.
>

All atoms whose are derived from IdataAtom are automatically added to
context.file. Look at IdataAtom::IdataAtom(Context &, vector<uint_8>). It's
weird if r193479 fixed the issue, but I might miss something.

I think you forgot to answer the question, are you ok with submitting the
> RoundTrip{YAML, Native} changes with the changes in the Driver removed,
> this would make all the tests pass and we can continue to figure out why
> those tests would fail, by enabling the RoundTrip code.
>

In order to answer that question, I'd want to see the patch, so I asked for
the latest patch. :)


> Once all the test results are fine, we could enable it completely.
>
> Looks like the IdataPass for COFF is really very complex!
>

Yes, the pass creates a pretty complex data structure. It's complex partly
because the file format is complex, but I'd suspect it's also because it's
a pass. I have an idea to move some code from IdataPass to the writer to
off-load some work that doesn't have to be done in a pass. I do not have
time to try now, but refactoring is on my TODO list.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131028/f3d84f91/attachment.html>