[PATCH] Add default parameter to MCSymbol accessors to control IsUsed

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 10:41:03 PDT 2015


Ahh, ok. Here is the NFC part (no problems with check-all on x86+arm).

I'll play around with the second part some more.

thanks!
vedant

-------------- next part --------------
A non-text attachment was scrubbed...
Name: MCSymbol-patch1-NFC.patch
Type: application/octet-stream
Size: 2856 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150825/28cf1ac2/attachment.obj>
-------------- next part --------------



> On Aug 25, 2015, at 6:36 AM, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:
> 
> With your patch we produce
> 
> 000000000000  00010000000a R_X86_64_32       0000000000000000 zed + 0
> 000000000004  00010000000a R_X86_64_32       0000000000000000 zed + 0
> 
> gas produces
> 
> 000000000000  00040000000a R_X86_64_32       0000000000000000 bar + 0
> 000000000004  00050000000a R_X86_64_32       0000000000000000 zed + 0
> 
> So we go from producing an error to producing an output that is
> different from gas, which is bad thing.
> 
> The part about not setting IsUsed unconditionally is still nice, so if
> you don't mind finishing up a NFC patch that just refines what is a
> use that would be great.
> 
> Cheers,
> Rafael
> 
> 
> 
> On 25 August 2015 at 09:26, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:
>> Taking a look.
>> 
>> On 21 August 2015 at 17:34, Vedant Kumar <vsk at apple.com> wrote:
>>> Ping
>>> 
>>> We were discussing "invalid reassignment to non-absolute variable" and whether we still need it around.
>>> 
>>> thanks
>>> vedant
>>> 
>>>> On Aug 13, 2015, at 9:31 AM, Vedant Kumar <vsk at apple.com> wrote:
>>>> 
>>>> Hi Rafael,
>>>> 
>>>> Sorry for the delay on this.
>>>> 
>>>>> I think that dates from the early days of MC. My understanding is that
>>>>> it was added just as a safety so that we have to reason about code
>>>>> like
>>>>> 
>>>>> foo = bar
>>>>> .long foo
>>>>> foo = zed
>>>>> .long foo
>>>>> 
>>>>> Should that produce two relocations with bar, two with foo or one each?
>>>> 
>>>> Here is what GAS does:
>>>> 
>>>> SYMBOL TABLE:
>>>> 00000000 l    d  .text        00000000 .text
>>>> 00000000 l    d  .data        00000000 .data
>>>> 00000000 l    d  .bss 00000000 .bss
>>>> 00000000         *UND*        00000000 bar
>>>> 00000000         *UND*        00000000 zed
>>>> 
>>>> RELOCATION RECORDS FOR [.text]:
>>>> OFFSET   TYPE              VALUE
>>>> 00000000 R_386_32          bar
>>>> 00000004 R_386_32          zed
>>>> 
>>>> Here is a different test that initially triggered this issue:
>>>> 
>>>>> foo = bar
>>>>> .long foo
>>>>> foo = bar
>>>>> .long foo
>>>> 
>>>> 
>>>> And here is GAS's output:
>>>> 
>>>> RELOCATION RECORDS FOR [.text]:
>>>> OFFSET   TYPE              VALUE
>>>> 00000000 R_386_32          bar
>>>> 00000004 R_386_32          bar
>>>> 
>>>> With the patch I suggested, both of these examples just work with llvm-mc. Here's what the relocation table looks like on Darwin for comparison:
>>>> 
>>>> Relocation information (__TEXT,__text) 2 entries
>>>> address  pcrel length extern type    scattered symbolnum/value
>>>> 00000004 0     2      1      0       0         1
>>>> 00000000 0     2      1      0       0         1
>>>> 
>>>> This looks sane to me. I've updated the patch to get rid of the "invalid reassignment to non-absolute variable" error and fixed the test cases. Could you review it for any issues?
>>>> 
>>>> thanks
>>>> vedant
>>>> 
>>>> <MCSymbol-patch1-NFC.patch><MCSymbol-patch2.patch>
>>> 



More information about the llvm-commits mailing list