[llvm-commits] lld update to add support for some hexagon relocations.

Michael Spencer bigcheesegs at gmail.com
Tue Oct 2 12:17:28 PDT 2012


On Tue, Oct 2, 2012 at 11:09 AM, Sid Manning <sidneym at codeaurora.org> wrote:
> On 09/28/12 12:22, Sid Manning wrote:
>>
>>
>> Attached are files that add support for the most basic relocation fixups
>> for hexagon.
>>
>> I added a new applyFixup method to take an int32_t, so instead of Kind
>> enums applyFixup can take enums defined in ELF.h.
>>
>> In the case of hexagon I moved code from ReferenceKinds.cpp to a new
>> file HexagonReference.cpp which will hold the Hexagon specific
>> relocation code.
>>
> Any comments on this patch?
>
>
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
> The Linux Foundation

> Index: lib/ReaderWriter/ELF/ReferenceKinds.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/ReferenceKinds.cpp	(revision 164787)
> +++ lib/ReaderWriter/ELF/ReferenceKinds.cpp	(working copy)
> @@ -82,82 +82,13 @@
>    return false;
>  }
>
> -
> -void KindHandler_x86::applyFixup(Kind kind, uint64_t addend,
> -                                    uint8_t *location, uint64_t fixupAddress,
> +void KindHandler_x86::applyFixup(int32_t reloc, uint64_t addend,
> +                                    uint8_t *location, uint64_t fixupAddress,
>                                      uint64_t targetAddress) {
> -  switch ((Kinds)kind) {
> -  case none:
> -    // do nothing
> -    break;
> -  case invalid:
> -    assert(0 && "invalid Reference Kind");
> -    break;
> -  }
> +  return;
> +  llvm_unreachable("Unimplemented: KindHandler_x86::applyFixup");
>  }

The llvm_unreachable is super unreachable :P. This should read:

  llvm_unreachable(...);
  return;

Or:

  return llvm_unreachable(...);

>
> -//===----------------------------------------------------------------------===//
> -//  KindHandler_hexagon
> -//  TODO: more to do here
> -//===----------------------------------------------------------------------===//
> -
> -KindHandler_hexagon::~KindHandler_hexagon() {
> -}
> -
> -Reference::Kind KindHandler_hexagon::stringToKind(StringRef str) {
> -  return llvm::StringSwitch<Reference::Kind>(str)
> -    .Case("none",                  none)
> -    .Default(invalid);
> -
> -  llvm_unreachable("invalid hexagon Reference kind");
> -}
> -
> -StringRef KindHandler_hexagon::kindToString(Reference::Kind kind) {
> -  switch ( (Kinds)kind ) {
> -    case invalid:
> -      return StringRef("invalid");
> -    case none:
> -      return StringRef("none");

No need for StringRef here. Same for a bunch of other places.

> -  }
> -  llvm_unreachable("invalid hexagon Reference kind");
> -}
> -
> -bool KindHandler_hexagon::isCallSite(Kind kind) {
> -  llvm_unreachable("Unimplemented: KindHandler_hexagon::isCallSite");
> -  return false;
> -}
> -
> -bool KindHandler_hexagon::isPointer(Kind kind) {
> -  llvm_unreachable("Unimplemented: KindHandler_hexagon::isPointer");
> -  return false;
> -}
> -
> -bool KindHandler_hexagon::isLazyImmediate(Kind kind) {
> -  llvm_unreachable("Unimplemented: KindHandler_hexagon::isLazyImmediate");
> -  return false;
> -}
> -
> -bool KindHandler_hexagon::isLazyTarget(Kind kind) {
> -  llvm_unreachable("Unimplemented: KindHandler_hexagon::isLazyTarget");
> -  return false;
> -}
> -
> -
> -void KindHandler_hexagon::applyFixup(Kind kind, uint64_t addend,
> -                                    uint8_t *location, uint64_t fixupAddress,
> -                                    uint64_t targetAddress) {
> -  switch ((Kinds)kind) {
> -  case none:
> -    // do nothing
> -    break;
> -  case invalid:
> -    llvm_unreachable("invalid Reference Kind");
> -    break;
> -  }
> -}
> -
> -
> -
>  } // namespace elf
>  } // namespace lld
>
> Index: lib/ReaderWriter/ELF/HexagonReference.cpp
> ===================================================================
> --- lib/ReaderWriter/ELF/HexagonReference.cpp	(revision 0)
> +++ lib/ReaderWriter/ELF/HexagonReference.cpp	(revision 0)
> @@ -0,0 +1,172 @@
> +//===- lib/ReaderWriter/ELF/ReferenceKinds.cpp ----------------------------===//
> +//
> +//                             The LLVM Linker
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +
> +#include "ReferenceKinds.h"
> +
> +#include "llvm/ADT/StringRef.h"
> +#include "llvm/ADT/StringSwitch.h"
> +
> +#include "llvm/Support/ErrorHandling.h"
> +#include "llvm/Support/ELF.h"
> +namespace lld {
> +namespace elf {
> +
> +//===----------------------------------------------------------------------===//
> +//  KindHandler_hexagon
> +//  TODO: more to do here
> +//===----------------------------------------------------------------------===//
> +
> +KindHandler_hexagon::~KindHandler_hexagon() {
> +}
> +
> +/// \brief The following relocation routines are derived from the
> +/// Hexagon ABI specification, Section 11.6: Relocation
> +/// Symbols used:
> +///  A: Added used to compute the value, r_addend
> +///  P: Place address of the field being relocated, r_offset
> +///  S: Value of the symbol whose index resides in the relocation entry.
> +
> +int reloc_NONE(uint8_t *location, uint64_t P, uint64_t S, uint64_t A) {
> +  return KindHandler_hexagon::NoError;
> +}
> +
> +/// \brief Word32_B22: 0x01ff3ffe : (S + A - P) >> 2 : Verify
> +int reloc_B22_PCREL(uint8_t *location, uint64_t P, uint64_t S, uint64_t A) {
> +  int32_t result = (uint32_t)(((S + A) - P)>>2);
> +  if ((result < 0x200000) && (result > -0x200000)) {
> +      result = ((result<<1) & 0x3ffe) | ((result<<3) & 0x01ff0000);
> +      *(uint32_t *)location |= result;

This assumes the target and host have the same endianess. Use somthing
equivelent to:

  *reinterpret_cast<u{little,big}32_t*>(location) = result |
*reinterpret_cast<const u{little,big}32_t*>(location);

This is true for all of the following. You will need to modify
KindHandler::makeHandler to get the endianess.

> +      return KindHandler_hexagon::NoError;
> +  }
> +  return KindHandler_hexagon::Overflow;
> +}
> +
> +/// \brief Word32_B15: 0x00df20fe : (S + A - P) >> 2 : Verify
> +int reloc_B15_PCREL(uint8_t *location, uint64_t P, uint64_t S, uint64_t A) {
> +  int32_t result = (uint32_t)(((S + A) - P)>>2);
> +  if ((result < 0x8000) && (result > -0x8000)) {
> +    result = ((result<<1) & 0x20fe) | ((result<<7) & 0x00df0000);
> +    *(uint32_t *)location |= result;
> +    return KindHandler_hexagon::NoError;
> +  }
> +  return KindHandler_hexagon::Overflow;
> +}
> +
> +/// \brief Word32_LO: 0x00c03fff : (S + A) : Truncate
> +int reloc_LO16(uint8_t *location, uint64_t P, uint64_t S, uint64_t A) {
> +  uint32_t result = (uint32_t)(S + A);
> +  result = ((result & 0x3fff) | ((result << 2) & 0x00c00000));
> +  *(uint32_t *)location |= result;
> +  return KindHandler_hexagon::NoError;
> +}
> +
> +/// \brief Word32_LO: 0x00c03fff : (S + A) >> 16 : Truncate
> +int reloc_HI16(uint8_t *location, uint64_t P, uint64_t S, uint64_t A) {
> +  uint32_t result = (uint32_t)((S + A)>>16);
> +  result = ((result & 0x3fff) | ((result << 2) & 0x00c00000));
> +  *(uint32_t *)location |= result;
> +  return KindHandler_hexagon::NoError;
> +}
> +
> +/// \brief Word32: 0xffffffff : (S + A) : Truncate
> +int reloc_32(uint8_t *location, uint64_t P, uint64_t S, uint64_t A) {
> +  uint32_t result = (uint32_t)(S + A);
> +  *(uint32_t *)location |= result;
> +  return KindHandler_hexagon::NoError;
> +}
> +
> +
> +void testfunc(void){}
> +KindHandler_hexagon::KindHandler_hexagon(){
> +

Remove undeeded blank lines.

> +  _fixupHandler[llvm::ELF::R_HEX_B22_PCREL] = reloc_B22_PCREL;
> +  _fixupHandler[llvm::ELF::R_HEX_B15_PCREL] = reloc_B15_PCREL;
> +  _fixupHandler[llvm::ELF::R_HEX_LO16]      = reloc_LO16;
> +  _fixupHandler[llvm::ELF::R_HEX_HI16]      = reloc_HI16;
> +  _fixupHandler[llvm::ELF::R_HEX_32]        = reloc_32;
> +
> +}
> +
> +Reference::Kind KindHandler_hexagon::stringToKind(StringRef str) {
> +  return llvm::StringSwitch<Reference::Kind>(str)
> +    .Case("none",                  none)
> +    .Case("R_HEX_B22_PCREL", llvm::ELF::R_HEX_B22_PCREL)
> +    .Case("R_HEX_B15_PCREL", llvm::ELF::R_HEX_B15_PCREL)
> +    .Case("R_HEX_LO16",      llvm::ELF::R_HEX_LO16)
> +    .Case("R_HEX_HI16",      llvm::ELF::R_HEX_HI16)
> +    .Case("R_HEX_32",        llvm::ELF::R_HEX_32)
> +    .Default(invalid);
> +  llvm_unreachable("invalid hexagon Reference kind");

StringSwitch doesn't work like this. llvm_unreachable statically cannot be
reached. If you want to disallow the invalid case, store the result of the
StringSwitch in a local and assert on it. Then return the value.

> +}
> +
> +StringRef KindHandler_hexagon::kindToString(Reference::Kind kind) {
> +  switch ( (int32_t)kind ) {

static_cast and no spaces after ( and before ).

> +    case llvm::ELF::R_HEX_B22_PCREL:
> +      return StringRef("R_HEX_B22_PCREL");
> +    case llvm::ELF::R_HEX_B15_PCREL:
> +      return StringRef("R_HEX_B15_PCREL");
> +    case llvm::ELF::R_HEX_LO16:
> +      return StringRef("R_HEX_LO16");
> +    case llvm::ELF::R_HEX_HI16:
> +      return StringRef("R_HEX_HI16");
> +    case llvm::ELF::R_HEX_32:
> +      return StringRef("R_HEX_32");
> +    default:
> +      return StringRef("none");

This block needs be deinteded two spaces.

> +  }
> +  llvm_unreachable("invalid hexagon Reference kind");

llvm_unreachable shouldn't be used when it is known that the switch statically
covers all cases.

> +}
> +
> +bool KindHandler_hexagon::isCallSite(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_hexagon::isCallSite");
> +  return false;
> +}
> +
> +bool KindHandler_hexagon::isPointer(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_hexagon::isPointer");
> +  return false;
> +}
> +
> +bool KindHandler_hexagon::isLazyImmediate(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_hexagon::isLazyImmediate");
> +  return false;
> +}
> +
> +bool KindHandler_hexagon::isLazyTarget(Kind kind) {
> +  llvm_unreachable("Unimplemented: KindHandler_hexagon::isLazyTarget");
> +  return false;
> +}
> +
> +void KindHandler_hexagon::applyFixup(int32_t reloc, uint64_t addend,
> +                                    uint8_t *location, uint64_t fixupAddress,
> +                                    uint64_t targetAddress) {

Line up with other arguments.

> +  int error;
> +  if (_fixupHandler[reloc])
> +  {

{ needs to be on previous line.

> +    error = (*_fixupHandler[reloc])(location,
> +                                   fixupAddress, targetAddress, addend);

Line up with other arguments.

> +
> +    switch ((RelocationError)error) {
> +    case NoError:
> +      return;
> +    case Overflow:
> +      llvm_unreachable("KindHandler_hexagon::applyFixup relocation overflow");

Is this actually unreachable? Or is this an error condition?

> +      return;
> +    }
> +  }
> +
> +  llvm_unreachable(
> +       "Unimplemented _fixupHandler KindHandler_hexagon::applyFixup");
> +}
> +
> +
> +
> +} // namespace elf
> +} // namespace lld
> Index: lib/ReaderWriter/ELF/CMakeLists.txt
> ===================================================================
> --- lib/ReaderWriter/ELF/CMakeLists.txt	(revision 164787)
> +++ lib/ReaderWriter/ELF/CMakeLists.txt	(working copy)
> @@ -2,5 +2,6 @@
>    ReaderELF.cpp
>    WriterELF.cpp
>    ReferenceKinds.cpp
> +  HexagonReference.cpp
>    WriterOptionsELF.cpp
>    )
> Index: lib/ReaderWriter/ELF/ReferenceKinds.h
> ===================================================================
> --- lib/ReaderWriter/ELF/ReferenceKinds.h	(revision 164787)
> +++ lib/ReaderWriter/ELF/ReferenceKinds.h	(working copy)
> @@ -7,11 +7,11 @@
>  //
>  //===----------------------------------------------------------------------===//
>
> -
>  #include "lld/Core/LLVM.h"
>  #include "lld/Core/Reference.h"
>  #include "lld/ReaderWriter/WriterELF.h"
>
> +#include <map>
>  #include <memory>
>
>  #ifndef LLD_READER_WRITER_ELF_REFERENCE_KINDS_H_
> @@ -39,8 +39,9 @@
>    virtual bool        isPointer(Kind) = 0;
>    virtual bool        isLazyImmediate(Kind) = 0;
>    virtual bool        isLazyTarget(Kind) = 0;
> -  virtual void        applyFixup(Kind kind, uint64_t addend, uint8_t *location,
> -                           uint64_t fixupAddress, uint64_t targetAddress) = 0;
> +  virtual void        applyFixup(int32_t reloc, uint64_t addend,
> +                             uint8_t *location,
> +                             uint64_t fixupAddress, uint64_t targetAddress) = 0;

Line up with other arguments.

>
>  protected:
>    KindHandler();
> @@ -49,21 +50,38 @@
>
>  class KindHandler_hexagon : public KindHandler {
>  public:
> +
> +// Note: Reference::Kinds are a another representation of
> +// relocation types, using negative values to represent architecture
> +// independent reference type.
> +// The positive values are the same ones defined in ELF.h and that
> +// is what we are using.

Doxygen comment.

>    enum Kinds {
> -    invalid,         // used to denote an error creating a Reference
> -    none,
> +    none            = llvm::ELF::R_HEX_NONE,
> +    invalid=255,         // used to denote an error creating a Reference

Spaces around =.

>    };
>
> +  enum RelocationError {
> +    NoError,
> +    Overflow
> +  };
> +
>    virtual ~KindHandler_hexagon();
> +  KindHandler_hexagon();
>    virtual Kind stringToKind(StringRef str);
>    virtual StringRef kindToString(Kind);
>    virtual bool isCallSite(Kind);
>    virtual bool isPointer(Kind);
>    virtual bool isLazyImmediate(Kind);
>    virtual bool isLazyTarget(Kind);
> -  virtual void applyFixup(Kind kind, uint64_t addend, uint8_t *location,
> -                  uint64_t fixupAddress, uint64_t targetAddress);
> +  virtual void applyFixup(int32_t reloc, uint64_t addend,
> +                          uint8_t *location,
> +                          uint64_t fixupAddress, uint64_t targetAddress);
>
> +private:
> +  std::map<int32_t, int (*)(uint8_t *location, uint64_t fixupAddress,
> +                 uint64_t targetAddress, uint64_t addend)> _fixupHandler;
> +
>  };
>
>
> @@ -81,8 +99,8 @@
>    virtual bool isPointer(Kind);
>    virtual bool isLazyImmediate(Kind);
>    virtual bool isLazyTarget(Kind);
> -  virtual void applyFixup(Kind kind, uint64_t addend, uint8_t *location,
> -                  uint64_t fixupAddress, uint64_t targetAddress);
> +  virtual void applyFixup(int32_t reloc, uint64_t addend, uint8_t *location,
> +                          uint64_t fixupAddress, uint64_t targetAddress);
>
>  };
>

- Michael Spencer



More information about the llvm-commits mailing list