[llvm-commits] [llvm] r120832 - /llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp

[Jim Grosbach]

On Dec 3, 2010, at 11:53 AM, Jim Grosbach wrote:

> 
> On Dec 3, 2010, at 11:40 AM, Jason W Kim wrote:
> 
>> Author: jasonwkim
>> Date: Fri Dec  3 13:40:23 2010
>> New Revision: 120832
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=120832&view=rev
>> Log:
>> fix ARM::fixup_arm_branch, cleanup, and share more code between ELF and Darwin
>> 
>> Modified:
>>   llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp
>> 
>> Modified: llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp?rev=120832&r1=120831&r2=120832&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMAsmBackend.cpp Fri Dec  3 13:40:23 2010
>> @@ -65,9 +65,16 @@
>>  default:
>>    llvm_unreachable("Unknown fixup kind!");
>>  case FK_Data_4:
>> +    return Value;
>>  case ARM::fixup_arm_movt_hi16:
>> -  case ARM::fixup_arm_movw_lo16:
>> +  case ARM::fixup_arm_movw_lo16: {
>> +    unsigned Hi4 = (Value & 0xF000) >> 12;
>> +    unsigned Lo12 = Value & 0x0FFF;
>> +    // inst{19-16} = Hi4;
>> +    // inst{11-0} = Lo12;
>> +    Value = (Hi4 << 16) | (Lo12);
>>    return Value;
>> +  }
>>  case ARM::fixup_arm_ldst_pcrel_12: {
>>    bool isAdd = true;
>>    // ARM PC-relative values are offset by 8.
>> @@ -96,7 +103,7 @@
>>  case ARM::fixup_arm_branch:
>>    // These values don't encode the low two bits since they're always zero.
>>    // Offset by 8 just as above.
>> -    return (Value - 8) >> 2;
>> +    return 0xFFFFFF & ((Value - 8) >> 2);
> 
> Why do we need to mask here? If the value doesn't fit in the range, that's an error, right? We should just assert, I think.
> 

After a moment's more thought.... this is for negative values? If so, never mind. :)

It wouldn't be a bad thing to add an explicit assert() that the absolute value of the unmasked value is in range.

> Likewise for the MOVW/MOVT values.
> 
>>  case ARM::fixup_arm_pcrel_10: {
>>    // Offset by 8 just as above.
>>    Value = Value - 8;
>> @@ -142,7 +149,7 @@
>>  }
>> };
>> 
>> -// Fixme: can we raise this to share code between Darwin and ELF?
>> +// Fixme: Raise this to share code between Darwin and ELF.
>> void ELFARMAsmBackend::ApplyFixup(const MCFixup &Fixup, MCDataFragment &DF,
>>                                  uint64_t Value) const {
>>  uint32_t Mask = 0;
>> @@ -150,32 +157,12 @@
>>  unsigned NumBytes = 4;
>>  Value = adjustFixupValue(Fixup.getKind(), Value);
>> 
>> -  switch (Fixup.getKind()) {
>> -  default: assert(0 && "Unsupported Fixup kind"); break;
>> -  case ARM::fixup_arm_branch: {
>> -    unsigned Lo24 = Value & 0xFFFFFF;
>> -    Mask = ~(0xFFFFFF);
>> -    Value = Lo24;
>> -  }; break;
>> -  case ARM::fixup_arm_movt_hi16:
>> -  case ARM::fixup_arm_movw_lo16: {
>> -    unsigned Hi4 = (Value & 0xF000) >> 12;
>> -    unsigned Lo12 = Value & 0x0FFF;
>> -    // inst{19-16} = Hi4;
>> -    // inst{11-0} = Lo12;
>> -    Value = (Hi4 << 16) | (Lo12);
>> -    Mask = ~(0xF0FFF);
>> -  }; break;
>> -  }
>> -
>>  assert((Fixup.getOffset() % NumBytes == 0)
>>         && "Offset mod NumBytes is nonzero!");
>>  // For each byte of the fragment that the fixup touches, mask in the
>>  // bits from the fixup value.
>>  // The Value has been "split up" into the appropriate bitfields above.
>> -  // Fixme: how to share code with the .td generated code?
>>  for (unsigned i = 0; i != NumBytes; ++i) {
>> -    DF.getContents()[Fixup.getOffset() + i] &= uint8_t(Mask >> (i * 8));
>>    DF.getContents()[Fixup.getOffset() + i] |= uint8_t(Value >> (i * 8));
>>  }
>> }
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>