[llvm-commits] [llvm] r80728 - /llvm/trunk/lib/Target/X86/X86CodeEmitter.cpp

[Daniel Dunbar]

On Tue, Sep 1, 2009 at 3:56 PM, Bruno Cardoso
Lopes<bruno.cardoso at gmail.com> wrote:
> Hi Daniel,
>
> On Tue, Sep 1, 2009 at 7:07 PM, Daniel Dunbar<daniel at zuster.org> wrote:
>> Author: ddunbar
>> Date: Tue Sep  1 17:07:00 2009
>> New Revision: 80728
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=80728&view=rev
>> Log:
>> Fix what I believe is a copy-n-pasto introduced in r78129.
>>  - Bruno, please check!!
>>
>> Modified:
>>    llvm/trunk/lib/Target/X86/X86CodeEmitter.cpp
>>
>> Modified: llvm/trunk/lib/Target/X86/X86CodeEmitter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86CodeEmitter.cpp?rev=80728&r1=80727&r2=80728&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86CodeEmitter.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86CodeEmitter.cpp Tue Sep  1 17:07:00 2009
>> @@ -340,18 +340,18 @@
>>   } else if (RelocOp->isSymbol()) {
>>     unsigned rt = Is64BitMode ?
>>       (IsPCRel ? X86::reloc_pcrel_word : X86::reloc_absolute_word_sext)
>> -      : (IsPCRel ? X86::reloc_picrel_word : X86::reloc_absolute_word);
>> +      : (IsPIC ? X86::reloc_picrel_word : X86::reloc_absolute_word);
>
> It's not a copy-n-pasto. The old behavior in r78129 was:
>
> unsigned rt = Is64BitMode ? X86::reloc_pcrel_word : X86::reloc_picrel_word;
>
> That is, to use reloc_picrel_word for "!Is64BitMode" even if we're not
> in PIC mode.

Ok. Disregarding that, is it correct? :)

Since the various encodings of a MachineOperand all amount to similar
concepts, it seems unlikely that they should emit different relocation
types. Also note that very similar code in other places, for example
for the MRM?m: encodings, uses this logic:
--
    unsigned rt = Is64BitMode ? X86::reloc_pcrel_word
      : (IsPIC ? X86::reloc_picrel_word : X86::reloc_absolute_word);
    if (Opcode == X86::MOV64mi32)
      rt = X86::reloc_absolute_word_sext;  // FIXME: add X86II flag?
    if (MO.isGlobal()) {
      bool NeedStub = isa<Function>(MO.getGlobal());
      bool Indirect = gvNeedsNonLazyPtr(MO, TM);
      emitGlobalAddress(MO.getGlobal(), rt, MO.getOffset(), 0,
                        NeedStub, Indirect);
--
which does check IsPIC. Similarly MRM?r encodings have a near copy of
this code that uses IsPIC.

One way or the other, I'm pretty sure that all copies of this code
should use the same logic, I'm just not sure which is right.

 - Daniel