Untested code

[Simon Atanasyan via llvm-commits]

On Wed, Apr 13, 2016 at 11:51 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> On 13 April 2016 at 16:50, Simon Atanasyan <simon at atanasyan.com> wrote:
>> On Wed, Apr 13, 2016 at 11:02 PM, Rafael Espíndola
>> <rafael.espindola at gmail.com> wrote:
>>> On 13 April 2016 at 10:58, Simon Atanasyan <simon at atanasyan.com> wrote:
>>>> On Wed, Apr 13, 2016 at 12:02 AM, Rafael Espíndola
>>>> <rafael.espindola at gmail.com> wrote:
>>>>> I think now there is only one if left untested (see the attached patch) :-)
>>>>
>>>> Attached patch adds test case and tries to fix the problem.
>>>>
>>>> _gp_disp designates offset between start of function and 'gp' pointer into GOT.
>>>> Usually you can see the following code in the beginning of a function:
>>>>
>>>> lui    $t0, %hi(_gp_disp)
>>>> addi   $t0, $t0, %lo(_gp_disp)
>>>>
>>>> When you calculate result of %hi(_gp_disp), you use %hi(_gp - P)
>>>> expression. For the first instruction "_gp - P" gives the distance
>>>> between the function and _gp. But for the second instruction you have
>>>> to add four byte offset %hi(_gp - P + 4). Code in the trunk
>>>> incorrectly calculates %lo(_gp_disp) as an absolute expression
>>>> %lo(_gp).
>>>>
>>>> I am not sure that the fix is ideal. So I would appreciate any
>>>> suggestions / objections.
>>>
>>> Is it guaranteed that _gp_disp can only ever be used as the first two
>>> instructions in a function? Is that restriction documented somewhere?
>>> If so, please add a link to the documentation.
>>
>> Yes. This pair of instructions is a standard function preamble. I will
>> try to find a link to spec and add it to the code.
>>
>>> Instead of changing InputSection.cpp, can you put the +4  in the addend?
>>
>> Where do you suggest to fix addend? It is impossible to do in the
>> getImplicitAddend routine because we do not pass SymbolBody to this
>> function. Without getImplicitAddend signature changing I can adjust
>> addend in the place where we call getImplicitAddend (Writer.cpp).
>
> At the call site in Writer.cpp should be fine.

Please take a look at http://reviews.llvm.org/D19115

Regards,
Simon