<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>There is a catch-22 here. Without the new passes it is hard for anyone to fix the other problems in lld that were causing the test case failures.</div><div><br></div><div>Can we do this in steps:</div><div>1) commit the code changes with the lines that use the two new passes commented out</div><div>2) owners of the broken areas can uncomment the code (enabling the passes) and fix their issues</div><div>3) the final commit is to enable the two new passes when all test cases pass </div><div><br></div><div>-Nick</div><br><div><div>On Oct 24, 2013, at 11:28 AM, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:</div><blockquote type="cite"><div dir="ltr">I reverted the patch because I had no choice other than that. Implementation quality of each port may vary, and probably ELF is the best at this moment because of its history, but intentionally submitting a patch that would break even "hello world" program on other port is simply unacceptable.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Oct 24, 2013 at 9:04 AM, Shankar Easwaran <span dir="ltr"><<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Nick,<br>
<br>
So I found the problem on Darwin when it tries to add a StubHelperAtom :-<br>
<br>
X86_64StubHelperAtom(const File &file, const Atom &helperCommon)<br>
: SimpleDefinedAtom(file) {<br>
this->addReference(<u></u>KindHandler_x86_64::<u></u>lazyImmediate, 1, nullptr, 0);<br>
this->addReference(<u></u>KindHandler_x86_64::ripRel32, 6, &helperCommon, 0);<br>
}<br>
<br>
A reference is being added to point to a nullptr. This has to be fixed to point to the right atom.<br>
<br>
Thanks<span class="HOEnZb"><font color="#888888"><br>
<br>
Shankar Easwaran</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
On 10/24/2013 10:31 AM, Shankar Easwaran wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 10/24/2013 12:23 AM, Rui Ueyama wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I did not expect you were going to submit your change with disabling so<br>
many tests. It disabled even pretty basic tests for COFF. This is really<br>
bad. Please don't do that.<br>
</blockquote>
The problem was not in the patch as what I see. The problem is because the relocation types have not been added by pecoff.<br>
<br>
ErrorOr<Reference::Kind><br>
PECOFFLinkingContext::<u></u>relocKindFromString(StringRef str) const {<br>
return make_error_code(<u></u>YamlReaderError::illegal_<u></u>value);<br>
}<br>
<br>
ErrorOr<std::string><br>
PECOFFLinkingContext::<u></u>stringFromRelocKind(Reference:<u></u>:Kind kind) const {<br>
return make_error_code(<u></u>YamlReaderError::illegal_<u></u>value);<br>
}<br>
<br>
These two are empty, this has to be fixed. This was the reason behind the failures for PECOFF. There are some more inherent assumptions that are being used which the new RoundTrip{YAML, Native} changes uncovered.<br>
<br>
*I am going to fix the ELF cases*, but I am going to leave it to the flavor owners for changes to their respective flavors as I dont have much knowledge on what kind of factors to be considered ( as these might be some assumptions that could break).<br>
<br>
Thanks<br>
<br>
Shankar Easwaran<br>
<br>
</blockquote>
<br>
<br>
-- <br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation<br>
<br>
</div></div></blockquote></div><br></div>
</blockquote></div><br></body></html>