<div dir="ltr">This looks good to me. Please take over this patch.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 7, 2016 at 6:58 AM, Simon Atanasyan <span dir="ltr"><<a href="mailto:simon@atanasyan.com" target="_blank">simon@atanasyan.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Here is the patch fixes all issues mentioned below. It also converts<br>
.MIPS.options section to synthetic one. The only failed test is the<br>
mips-options-r.test. The linker creates .MIPS.options section, but<br>
does not create a corresponding section symbol. I'm going to<br>
investigate it.<br>
<br>
Rui, what do you think about the patch?<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Nov 7, 2016 at 12:40 AM, Simon Atanasyan <<a href="mailto:simon@atanasyan.com">simon@atanasyan.com</a>> wrote:<br>
> atanasyan added inline comments.<br>
><br>
> ================<br>
> Comment at: ELF/InputFiles.cpp:166<br>
>      return MipsOptions->Reginfo->ri_gp_<wbr>value;<br>
> -  if (!ELFT::Is64Bits && MipsReginfo && MipsReginfo->Reginfo)<br>
> -    return MipsReginfo->Reginfo->ri_gp_<wbr>value;<br>
> ----------------<br>
> When we calculate R_MIPS_GPREL16/32 relocations we need to know `GP0` value stored in the .MIPS.options or .reginfo sections (take a look at the `computeAddend` routine). Each file might have it's own value. Usually `GP0` is equal to zero but it is not always true. After this change `getMipsGp0()` does not take in account `GP0` value stored in the .reginfo section and the `mips-gprel32-relocs-gp0.s` test case starts to fail.<br>
><br>
> To solve the problem we can add `MipsGp0` field to the `ObjectFile` class and retrieve and save `GP0` value to this field in the say `ObjectFile::<wbr>initializeSections` function.<br>
><br>
> If you want I can work on that and convert MipsOptionsInputSection to synthetic input section too. It looks like it is better to convert .reginfo and .MIPS.options sections in a single patch.<br>
><br>
><br>
> ================<br>
> Comment at: ELF/SyntheticSections.cpp:110<br>
> +    if (Config->Relocatable && RI->ri_gp_value)<br>
> +      error("supported non-zero ri_gp_value");<br>
> +    Reginfo.ri_gprmask |= RI->ri_gprmask;<br>
> ----------------<br>
> - s/supported/unsupported/<br>
> - it is not very important, but it would be nice to continue to show a file's name in this error message. maybe add an argument to the `getSectionContents` routine to be able to pass a //validate// function.<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D26327" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D26327</a><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Simon Atanasyan<br>
</font></span></blockquote></div><br></div>