[llvm] [JITLink][AArch32] Add dynamic lookup for relocation fixup infos (PR #71649)

Stefan Gränitz via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 24 07:11:55 PST 2023


https://github.com/weliveindetail updated https://github.com/llvm/llvm-project/pull/71649

>From 7998df6c5817071eb401d3d7d45983f4ea7d0151 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Tue, 7 Nov 2023 18:50:35 +0100
Subject: [PATCH 1/3] [JITLink][AArch32] Add dynamic lookup for relocation
 fixup infos

---
 .../llvm/ExecutionEngine/JITLink/aarch32.h    |  77 ++++++--
 .../ExecutionEngine/JITLink/ELF_aarch32.cpp   |   2 +
 llvm/lib/ExecutionEngine/JITLink/aarch32.cpp  | 165 +++++++++++-------
 3 files changed, 169 insertions(+), 75 deletions(-)

diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
index 94669e0bceb836d..04ed64a04a25905 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
@@ -98,6 +98,7 @@ enum EdgeKind_aarch32 : Edge::Kind {
   Thumb_MovtPrel,
 
   LastThumbRelocation = Thumb_MovtPrel,
+  LastRelocation = LastThumbRelocation,
 };
 
 /// Flags enum for AArch32-specific symbol properties
@@ -163,49 +164,81 @@ struct HalfWords {
   const uint16_t Lo; // Second halfword
 };
 
-/// Collection of named constants per fixup kind. It may contain but is not
-/// limited to the following entries:
+/// Initialize the lookup table for dynamic FixupInfo checks, e.g. checkOpcode()
+void populateFixupInfos();
+
+/// FixupInfo base class is required for dynamic lookups.
+struct FixupInfoBase {
+  virtual ~FixupInfoBase() {}
+};
+
+/// FixupInfo checks for Arm edge kinds work on 32-bit words
+struct FixupInfoArm : public FixupInfoBase {
+  bool (*checkOpcode)(uint32_t Wd) = nullptr;
+};
+
+/// FixupInfo check for Thumb32 edge kinds work on a pair of 16-bit halfwords
+struct FixupInfoThumb : public FixupInfoBase {
+  bool (*checkOpcode)(uint16_t Hi, uint16_t Lo) = nullptr;
+};
+
+/// Collection of named constants per fixup kind
 ///
+/// Mandatory entries:
 ///   Opcode      - Values of the op-code bits in the instruction, with
 ///                 unaffected bits nulled
 ///   OpcodeMask  - Mask with all bits set that encode the op-code
+///
+/// Other common entries:
 ///   ImmMask     - Mask with all bits set that encode the immediate value
 ///   RegMask     - Mask with all bits set that encode the register
 ///
+/// Specializations can add further custom fields without restrictions.
+///
 template <EdgeKind_aarch32 Kind> struct FixupInfo {};
 
-template <> struct FixupInfo<Arm_Jump24> {
+namespace {
+struct FixupInfoArmBranch : public FixupInfoArm {
   static constexpr uint32_t Opcode = 0x0a000000;
-  static constexpr uint32_t OpcodeMask = 0x0f000000;
   static constexpr uint32_t ImmMask = 0x00ffffff;
-  static constexpr uint32_t Unconditional = 0xe0000000;
-  static constexpr uint32_t CondMask = 0xe0000000; // excluding BLX bit
 };
+} // namespace
 
-template <> struct FixupInfo<Arm_Call> : public FixupInfo<Arm_Jump24> {
+template <> struct FixupInfo<Arm_Jump24> : public FixupInfoArmBranch {
+  static constexpr uint32_t OpcodeMask = 0x0f000000;
+};
+
+template <> struct FixupInfo<Arm_Call> : public FixupInfoArmBranch {
   static constexpr uint32_t OpcodeMask = 0x0e000000;
+  static constexpr uint32_t CondMask = 0xe0000000; // excluding BLX bit
+  static constexpr uint32_t Unconditional = 0xe0000000;
   static constexpr uint32_t BitH = 0x01000000;
   static constexpr uint32_t BitBlx = 0x10000000;
 };
 
-template <> struct FixupInfo<Arm_MovtAbs> {
-  static constexpr uint32_t Opcode = 0x03400000;
+namespace {
+struct FixupInfoArmMov : public FixupInfoArm {
   static constexpr uint32_t OpcodeMask = 0x0ff00000;
   static constexpr uint32_t ImmMask = 0x000f0fff;
   static constexpr uint32_t RegMask = 0x0000f000;
 };
+} // namespace
+
+template <> struct FixupInfo<Arm_MovtAbs> : public FixupInfoArmMov {
+  static constexpr uint32_t Opcode = 0x03400000;
+};
 
-template <> struct FixupInfo<Arm_MovwAbsNC> : public FixupInfo<Arm_MovtAbs> {
+template <> struct FixupInfo<Arm_MovwAbsNC> : public FixupInfoArmMov {
   static constexpr uint32_t Opcode = 0x03000000;
 };
 
-template <> struct FixupInfo<Thumb_Jump24> {
+template <> struct FixupInfo<Thumb_Jump24> : public FixupInfoThumb {
   static constexpr HalfWords Opcode{0xf000, 0x9000};
   static constexpr HalfWords OpcodeMask{0xf800, 0x9000};
   static constexpr HalfWords ImmMask{0x07ff, 0x2fff};
 };
 
-template <> struct FixupInfo<Thumb_Call> {
+template <> struct FixupInfo<Thumb_Call> : public FixupInfoThumb {
   static constexpr HalfWords Opcode{0xf000, 0xc000};
   static constexpr HalfWords OpcodeMask{0xf800, 0xc000};
   static constexpr HalfWords ImmMask{0x07ff, 0x2fff};
@@ -213,15 +246,27 @@ template <> struct FixupInfo<Thumb_Call> {
   static constexpr uint16_t LoBitNoBlx = 0x1000;
 };
 
-template <> struct FixupInfo<Thumb_MovtAbs> {
-  static constexpr HalfWords Opcode{0xf2c0, 0x0000};
+namespace {
+struct FixupInfoThumbMov : public FixupInfoThumb {
   static constexpr HalfWords OpcodeMask{0xfbf0, 0x8000};
   static constexpr HalfWords ImmMask{0x040f, 0x70ff};
   static constexpr HalfWords RegMask{0x0000, 0x0f00};
 };
+} // namespace
 
-template <>
-struct FixupInfo<Thumb_MovwAbsNC> : public FixupInfo<Thumb_MovtAbs> {
+template <> struct FixupInfo<Thumb_MovtAbs> : public FixupInfoThumbMov {
+  static constexpr HalfWords Opcode{0xf2c0, 0x0000};
+};
+
+template <> struct FixupInfo<Thumb_MovtPrel> : public FixupInfoThumbMov {
+  static constexpr HalfWords Opcode{0xf2c0, 0x0000};
+};
+
+template <> struct FixupInfo<Thumb_MovwAbsNC> : public FixupInfoThumbMov {
+  static constexpr HalfWords Opcode{0xf240, 0x0000};
+};
+
+template <> struct FixupInfo<Thumb_MovwPrelNC> : public FixupInfoThumbMov {
   static constexpr HalfWords Opcode{0xf240, 0x0000};
 };
 
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
index 132989fcbce0213..b6c0fe6d2ef59af 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
@@ -232,6 +232,8 @@ createLinkGraphFromELFObject_aarch32(MemoryBufferRef ObjectBuffer) {
            << ObjectBuffer.getBufferIdentifier() << "...\n";
   });
 
+  aarch32::populateFixupInfos();
+
   auto ELFObj = ObjectFile::createELFObjectFile(ObjectBuffer);
   if (!ELFObj)
     return ELFObj.takeError();
diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
index b80b12ed245192f..8d575a447177489 100644
--- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
@@ -189,6 +189,8 @@ int64_t decodeRegMovtA1MovwA2(uint64_t Value) {
   return Rd4;
 }
 
+namespace {
+
 /// 32-bit Thumb instructions are stored as two little-endian halfwords.
 /// An instruction at address A encodes bytes A+1, A in the first halfword (Hi),
 /// followed by bytes A+3, A+2 in the second halfword (Lo).
@@ -224,7 +226,6 @@ struct WritableArmRelocation {
 };
 
 struct ArmRelocation {
-
   ArmRelocation(const char *FixupPtr)
       : Wd{*reinterpret_cast<const support::ulittle32_t *>(FixupPtr)} {}
 
@@ -248,15 +249,98 @@ Error makeUnexpectedOpcodeError(const LinkGraph &G, const ArmRelocation &R,
               static_cast<uint32_t>(R.Wd), G.getEdgeKindName(Kind)));
 }
 
-template <EdgeKind_aarch32 Kind> bool checkOpcode(const ThumbRelocation &R) {
-  uint16_t Hi = R.Hi & FixupInfo<Kind>::OpcodeMask.Hi;
-  uint16_t Lo = R.Lo & FixupInfo<Kind>::OpcodeMask.Lo;
-  return Hi == FixupInfo<Kind>::Opcode.Hi && Lo == FixupInfo<Kind>::Opcode.Lo;
+static auto &getFixupInfoTable() {
+  static constexpr size_t Items = LastRelocation + 1;
+  static std::array<std::unique_ptr<FixupInfoBase>, Items> FixupInfoTable;
+  return FixupInfoTable;
+}
+
+template <EdgeKind_aarch32 K> constexpr bool isArm() {
+  return FirstArmRelocation <= K && K <= LastArmRelocation;
+}
+template <EdgeKind_aarch32 K> constexpr bool isThumb() {
+  return FirstThumbRelocation <= K && K <= LastThumbRelocation;
+}
+
+template <EdgeKind_aarch32 K> constexpr bool hasOpcode(...) { return false; }
+template <EdgeKind_aarch32 K, auto _ = FixupInfo<K>::Opcode>
+constexpr bool hasOpcode(int) {
+  return true;
+}
+
+template <EdgeKind_aarch32 K> static bool checkOpcodeArm(uint32_t Wd) {
+  return (Wd & FixupInfo<K>::OpcodeMask) == FixupInfo<K>::Opcode;
+}
+
+template <EdgeKind_aarch32 K>
+static bool checkOpcodeThumb(uint16_t Hi, uint16_t Lo) {
+  return (Hi & FixupInfo<K>::OpcodeMask.Hi) == FixupInfo<K>::Opcode.Hi &&
+         (Lo & FixupInfo<K>::OpcodeMask.Lo) == FixupInfo<K>::Opcode.Lo;
+}
+
+template <EdgeKind_aarch32 K>
+static std::unique_ptr<FixupInfoBase> initFixupInfo() {
+  auto Entry = std::make_unique<FixupInfo<K>>();
+  if constexpr (hasOpcode<K>(0)) {
+    if constexpr (isArm<K>())
+      Entry->checkOpcode = checkOpcodeArm<K>;
+    else if constexpr (isThumb<K>())
+      Entry->checkOpcode = checkOpcodeThumb<K>;
+    else
+      llvm_unreachable("Visited edge kinds must either be Arm or Thumb");
+  }
+  return Entry;
+}
+
+template <EdgeKind_aarch32 K, EdgeKind_aarch32 LastK, typename TableT>
+void populateFixupInfos(TableT &Table) {
+  assert(K < Table.size() && "Index out of range");
+  assert(Table.at(K) == nullptr && "Initialized entries are immutable");
+  Table[K] = initFixupInfo<K>();
+  if constexpr (K < LastK) {
+    constexpr auto Next = static_cast<EdgeKind_aarch32>(K + 1);
+    populateFixupInfos<Next, LastK>(Table);
+  }
+}
+} // namespace
+
+void populateFixupInfos() {
+  static std::once_flag Flag;
+  std::call_once(Flag, []() {
+    auto &Table = getFixupInfoTable();
+    populateFixupInfos<FirstArmRelocation, LastArmRelocation>(Table);
+    populateFixupInfos<FirstThumbRelocation, LastThumbRelocation>(Table);
+  });
+}
+
+template <typename TargetModeSubclass>
+static const TargetModeSubclass *const getDynFixupInfo(Edge::Kind K) {
+  assert(K >= Edge::FirstRelocation && "Invalid value for edge kind");
+  assert(K <= LastRelocation && "Invalid value for edge kind");
+  assert(getFixupInfoTable().at(K) && "Please call populateFixupInfos() first");
+  return static_cast<TargetModeSubclass *>(getFixupInfoTable().at(K).get());
+}
+
+static bool checkOpcode(const ArmRelocation &R, Edge::Kind Kind) {
+  assert(Kind >= FirstArmRelocation && Kind <= LastArmRelocation &&
+         "Edge kind is no Arm relocation");
+  const FixupInfoArm *const Info = getDynFixupInfo<FixupInfoArm>(Kind);
+  if (LLVM_LIKELY(Info) && LLVM_LIKELY(Info->checkOpcode))
+    return Info->checkOpcode(R.Wd);
+  LLVM_DEBUG(dbgs() << "Can not perform Opcode check for aarch32 edge kind "
+                    << getEdgeKindName(Kind));
+  return true;
 }
 
-template <EdgeKind_aarch32 Kind> bool checkOpcode(const ArmRelocation &R) {
-  uint32_t Wd = R.Wd & FixupInfo<Kind>::OpcodeMask;
-  return Wd == FixupInfo<Kind>::Opcode;
+static bool checkOpcode(const ThumbRelocation &R, Edge::Kind Kind) {
+  assert(Kind >= FirstThumbRelocation && Kind <= LastThumbRelocation &&
+         "Edge kind is no Thumb relocation");
+  const FixupInfoThumb *const Info = getDynFixupInfo<FixupInfoThumb>(Kind);
+  if (LLVM_LIKELY(Info) && LLVM_LIKELY(Info->checkOpcode))
+    return Info->checkOpcode(R.Hi, R.Lo);
+  LLVM_DEBUG(dbgs() << "Can not perform Opcode check for aarch32 edge kind "
+                    << getEdgeKindName(Kind));
+  return true;
 }
 
 template <EdgeKind_aarch32 Kind>
@@ -325,26 +409,16 @@ Expected<int64_t> readAddendData(LinkGraph &G, Block &B, Edge::OffsetT Offset,
 Expected<int64_t> readAddendArm(LinkGraph &G, Block &B, Edge::OffsetT Offset,
                                 Edge::Kind Kind) {
   ArmRelocation R(B.getContent().data() + Offset);
+  if (!checkOpcode(R, Kind))
+    return makeUnexpectedOpcodeError(G, R, Kind);
 
   switch (Kind) {
   case Arm_Call:
-    if (!checkOpcode<Arm_Call>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
-    return decodeImmBA1BlA1BlxA2(R.Wd);
-
   case Arm_Jump24:
-    if (!checkOpcode<Arm_Jump24>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     return decodeImmBA1BlA1BlxA2(R.Wd);
 
-  case Arm_MovwAbsNC:
-    if (!checkOpcode<Arm_MovwAbsNC>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
-    return decodeImmMovtA1MovwA2(R.Wd);
-
   case Arm_MovtAbs:
-    if (!checkOpcode<Arm_MovtAbs>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
+  case Arm_MovwAbsNC:
     return decodeImmMovtA1MovwA2(R.Wd);
 
   default:
@@ -358,33 +432,23 @@ Expected<int64_t> readAddendArm(LinkGraph &G, Block &B, Edge::OffsetT Offset,
 Expected<int64_t> readAddendThumb(LinkGraph &G, Block &B, Edge::OffsetT Offset,
                                   Edge::Kind Kind, const ArmConfig &ArmCfg) {
   ThumbRelocation R(B.getContent().data() + Offset);
+  if (!checkOpcode(R, Kind))
+    return makeUnexpectedOpcodeError(G, R, Kind);
 
   switch (Kind) {
   case Thumb_Call:
-    if (!checkOpcode<Thumb_Call>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
+  case Thumb_Jump24:
     return LLVM_LIKELY(ArmCfg.J1J2BranchEncoding)
                ? decodeImmBT4BlT1BlxT2_J1J2(R.Hi, R.Lo)
                : decodeImmBT4BlT1BlxT2(R.Hi, R.Lo);
 
-  case Thumb_Jump24:
-    if (!checkOpcode<Thumb_Jump24>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
-    return LLVM_LIKELY(ArmCfg.J1J2BranchEncoding)
-                  ? decodeImmBT4BlT1BlxT2_J1J2(R.Hi, R.Lo)
-                  : decodeImmBT4BlT1BlxT2(R.Hi, R.Lo);
-
   case Thumb_MovwAbsNC:
   case Thumb_MovwPrelNC:
-    if (!checkOpcode<Thumb_MovwAbsNC>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     // Initial addend is interpreted as a signed value
     return SignExtend64<16>(decodeImmMovtT1MovwT3(R.Hi, R.Lo));
 
   case Thumb_MovtAbs:
   case Thumb_MovtPrel:
-    if (!checkOpcode<Thumb_MovtAbs>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     // Initial addend is interpreted as a signed value
     return SignExtend64<16>(decodeImmMovtT1MovwT3(R.Hi, R.Lo));
 
@@ -446,6 +510,9 @@ Error applyFixupData(LinkGraph &G, Block &B, const Edge &E) {
 Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) {
   WritableArmRelocation R(B.getAlreadyMutableContent().data() + E.getOffset());
   Edge::Kind Kind = E.getKind();
+  if (!checkOpcode(R, Kind))
+    return makeUnexpectedOpcodeError(G, R, Kind);
+
   uint64_t FixupAddress = (B.getAddress() + E.getOffset()).getValue();
   int64_t Addend = E.getAddend();
   Symbol &TargetSymbol = E.getTarget();
@@ -453,8 +520,6 @@ Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) {
 
   switch (Kind) {
   case Arm_Jump24: {
-    if (!checkOpcode<Arm_Jump24>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     if (hasTargetFlags(TargetSymbol, ThumbSymbol))
       return make_error<JITLinkError>("Branch relocation needs interworking "
                                       "stub when bridging to Thumb: " +
@@ -469,8 +534,6 @@ Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) {
     return Error::success();
   }
   case Arm_Call: {
-    if (!checkOpcode<Arm_Call>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     if ((R.Wd & FixupInfo<Arm_Call>::CondMask) !=
         FixupInfo<Arm_Call>::Unconditional)
       return make_error<JITLinkError>("Relocation expects an unconditional "
@@ -501,15 +564,11 @@ Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) {
     return Error::success();
   }
   case Arm_MovwAbsNC: {
-    if (!checkOpcode<Arm_MovwAbsNC>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     uint16_t Value = (TargetAddress + Addend) & 0xffff;
     writeImmediate<Arm_MovwAbsNC>(R, encodeImmMovtA1MovwA2(Value));
     return Error::success();
   }
   case Arm_MovtAbs: {
-    if (!checkOpcode<Arm_MovtAbs>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     uint16_t Value = ((TargetAddress + Addend) >> 16) & 0xffff;
     writeImmediate<Arm_MovtAbs>(R, encodeImmMovtA1MovwA2(Value));
     return Error::success();
@@ -526,8 +585,10 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E,
                       const ArmConfig &ArmCfg) {
   WritableThumbRelocation R(B.getAlreadyMutableContent().data() +
                             E.getOffset());
-
   Edge::Kind Kind = E.getKind();
+  if (!checkOpcode(R, Kind))
+    return makeUnexpectedOpcodeError(G, R, Kind);
+
   uint64_t FixupAddress = (B.getAddress() + E.getOffset()).getValue();
   int64_t Addend = E.getAddend();
   Symbol &TargetSymbol = E.getTarget();
@@ -535,8 +596,6 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E,
 
   switch (Kind) {
   case Thumb_Jump24: {
-    if (!checkOpcode<Thumb_Jump24>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     if (!hasTargetFlags(TargetSymbol, ThumbSymbol))
       return make_error<JITLinkError>("Branch relocation needs interworking "
                                       "stub when bridging to ARM: " +
@@ -557,9 +616,6 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E,
   }
 
   case Thumb_Call: {
-    if (!checkOpcode<Thumb_Call>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
-
     int64_t Value = TargetAddress - FixupAddress + Addend;
 
     // The call instruction itself is Thumb. The call destination can either be
@@ -596,32 +652,23 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E,
   }
 
   case Thumb_MovwAbsNC: {
-    if (!checkOpcode<Thumb_MovwAbsNC>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     uint16_t Value = (TargetAddress + Addend) & 0xffff;
     writeImmediate<Thumb_MovwAbsNC>(R, encodeImmMovtT1MovwT3(Value));
     return Error::success();
   }
-
   case Thumb_MovtAbs: {
-    if (!checkOpcode<Thumb_MovtAbs>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     uint16_t Value = ((TargetAddress + Addend) >> 16) & 0xffff;
     writeImmediate<Thumb_MovtAbs>(R, encodeImmMovtT1MovwT3(Value));
     return Error::success();
   }
   case Thumb_MovwPrelNC: {
-    if (!checkOpcode<Thumb_MovwAbsNC>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     uint16_t Value = ((TargetAddress + Addend - FixupAddress) & 0xffff);
-    writeImmediate<Thumb_MovwAbsNC>(R, encodeImmMovtT1MovwT3(Value));
+    writeImmediate<Thumb_MovwPrelNC>(R, encodeImmMovtT1MovwT3(Value));
     return Error::success();
   }
   case Thumb_MovtPrel: {
-    if (!checkOpcode<Thumb_MovtAbs>(R))
-      return makeUnexpectedOpcodeError(G, R, Kind);
     uint16_t Value = (((TargetAddress + Addend - FixupAddress) >> 16) & 0xffff);
-    writeImmediate<Thumb_MovtAbs>(R, encodeImmMovtT1MovwT3(Value));
+    writeImmediate<Thumb_MovtPrel>(R, encodeImmMovtT1MovwT3(Value));
     return Error::success();
   }
 

>From a6512e2adf6666d4addf3e216518b65ee86186ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Sun, 12 Nov 2023 22:24:40 +0100
Subject: [PATCH 2/3] Fix deduction for definition of FixupInfo<Kind>::Opcode

---
 llvm/lib/ExecutionEngine/JITLink/aarch32.cpp | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
index 8d575a447177489..a8c829246d66e3f 100644
--- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
@@ -264,9 +264,9 @@ template <EdgeKind_aarch32 K> constexpr bool isThumb() {
 
 template <EdgeKind_aarch32 K> constexpr bool hasOpcode(...) { return false; }
 template <EdgeKind_aarch32 K, auto _ = FixupInfo<K>::Opcode>
-constexpr bool hasOpcode(int) {
-  return true;
-}
+constexpr bool hasOpcode(int) { return true; }
+template <EdgeKind_aarch32 K, auto _ = FixupInfo<K>::Opcode.Lo>
+constexpr bool hasOpcode(unsigned) { return true; }
 
 template <EdgeKind_aarch32 K> static bool checkOpcodeArm(uint32_t Wd) {
   return (Wd & FixupInfo<K>::OpcodeMask) == FixupInfo<K>::Opcode;
@@ -282,12 +282,11 @@ template <EdgeKind_aarch32 K>
 static std::unique_ptr<FixupInfoBase> initFixupInfo() {
   auto Entry = std::make_unique<FixupInfo<K>>();
   if constexpr (hasOpcode<K>(0)) {
+    static_assert(isArm<K>() != isThumb<K>(), "Classes are mutually exclusive");
     if constexpr (isArm<K>())
       Entry->checkOpcode = checkOpcodeArm<K>;
-    else if constexpr (isThumb<K>())
+    if constexpr (isThumb<K>())
       Entry->checkOpcode = checkOpcodeThumb<K>;
-    else
-      llvm_unreachable("Visited edge kinds must either be Arm or Thumb");
   }
   return Entry;
 }

>From c7ef2b3f8f596efc406f4a0e365dc7027ba46a65 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Fri, 24 Nov 2023 15:59:33 +0100
Subject: [PATCH 3/3] Store DynFixupInfos as ManagedStatic to achieve lazy init

---
 .../llvm/ExecutionEngine/JITLink/aarch32.h    |   4 +-
 .../ExecutionEngine/JITLink/ELF_aarch32.cpp   |   2 -
 llvm/lib/ExecutionEngine/JITLink/aarch32.cpp  | 140 +++++++++---------
 .../ExecutionEngine/JITLink/AArch32Tests.cpp  |  23 +++
 4 files changed, 98 insertions(+), 71 deletions(-)

diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
index 04ed64a04a25905..6d48e7e4f5074ff 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
@@ -164,11 +164,9 @@ struct HalfWords {
   const uint16_t Lo; // Second halfword
 };
 
-/// Initialize the lookup table for dynamic FixupInfo checks, e.g. checkOpcode()
-void populateFixupInfos();
-
 /// FixupInfo base class is required for dynamic lookups.
 struct FixupInfoBase {
+  static const FixupInfoBase *getDynFixupInfo(Edge::Kind K);
   virtual ~FixupInfoBase() {}
 };
 
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
index b6c0fe6d2ef59af..132989fcbce0213 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
@@ -232,8 +232,6 @@ createLinkGraphFromELFObject_aarch32(MemoryBufferRef ObjectBuffer) {
            << ObjectBuffer.getBufferIdentifier() << "...\n";
   });
 
-  aarch32::populateFixupInfos();
-
   auto ELFObj = ObjectFile::createELFObjectFile(ObjectBuffer);
   if (!ELFObj)
     return ELFObj.takeError();
diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
index a8c829246d66e3f..f47e05d52604c6a 100644
--- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ExecutionEngine/JITLink/JITLink.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Support/Endian.h"
+#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MathExtras.h"
 
 #define DEBUG_TYPE "jitlink"
@@ -249,12 +250,6 @@ Error makeUnexpectedOpcodeError(const LinkGraph &G, const ArmRelocation &R,
               static_cast<uint32_t>(R.Wd), G.getEdgeKindName(Kind)));
 }
 
-static auto &getFixupInfoTable() {
-  static constexpr size_t Items = LastRelocation + 1;
-  static std::array<std::unique_ptr<FixupInfoBase>, Items> FixupInfoTable;
-  return FixupInfoTable;
-}
-
 template <EdgeKind_aarch32 K> constexpr bool isArm() {
   return FirstArmRelocation <= K && K <= LastArmRelocation;
 }
@@ -278,68 +273,81 @@ static bool checkOpcodeThumb(uint16_t Hi, uint16_t Lo) {
          (Lo & FixupInfo<K>::OpcodeMask.Lo) == FixupInfo<K>::Opcode.Lo;
 }
 
-template <EdgeKind_aarch32 K>
-static std::unique_ptr<FixupInfoBase> initFixupInfo() {
-  auto Entry = std::make_unique<FixupInfo<K>>();
-  if constexpr (hasOpcode<K>(0)) {
-    static_assert(isArm<K>() != isThumb<K>(), "Classes are mutually exclusive");
-    if constexpr (isArm<K>())
-      Entry->checkOpcode = checkOpcodeArm<K>;
-    if constexpr (isThumb<K>())
-      Entry->checkOpcode = checkOpcodeThumb<K>;
+class FixupInfoTable {
+  static constexpr size_t Items = LastRelocation + 1;
+
+public:
+  FixupInfoTable() {
+    populateEntries<FirstArmRelocation, LastArmRelocation>();
+    populateEntries<FirstThumbRelocation, LastThumbRelocation>();
   }
-  return Entry;
-}
 
-template <EdgeKind_aarch32 K, EdgeKind_aarch32 LastK, typename TableT>
-void populateFixupInfos(TableT &Table) {
-  assert(K < Table.size() && "Index out of range");
-  assert(Table.at(K) == nullptr && "Initialized entries are immutable");
-  Table[K] = initFixupInfo<K>();
-  if constexpr (K < LastK) {
-    constexpr auto Next = static_cast<EdgeKind_aarch32>(K + 1);
-    populateFixupInfos<Next, LastK>(Table);
+  const FixupInfoBase *getEntry(Edge::Kind K) {
+    assert(K < Data.size() && "Index out of bounds");
+    return Data.at(K).get();
   }
-}
-} // namespace
 
-void populateFixupInfos() {
-  static std::once_flag Flag;
-  std::call_once(Flag, []() {
-    auto &Table = getFixupInfoTable();
-    populateFixupInfos<FirstArmRelocation, LastArmRelocation>(Table);
-    populateFixupInfos<FirstThumbRelocation, LastThumbRelocation>(Table);
-  });
-}
+private:
+  template <EdgeKind_aarch32 K, EdgeKind_aarch32 LastK>
+  void populateEntries() {
+    assert(K < Data.size() && "Index out of range");
+    assert(Data.at(K) == nullptr && "Initialized entries are immutable");
+    Data[K] = initEntry<K>();
+    if constexpr (K < LastK) {
+      constexpr auto Next = static_cast<EdgeKind_aarch32>(K + 1);
+      populateEntries<Next, LastK>();
+    }
+  }
 
-template <typename TargetModeSubclass>
-static const TargetModeSubclass *const getDynFixupInfo(Edge::Kind K) {
-  assert(K >= Edge::FirstRelocation && "Invalid value for edge kind");
-  assert(K <= LastRelocation && "Invalid value for edge kind");
-  assert(getFixupInfoTable().at(K) && "Please call populateFixupInfos() first");
-  return static_cast<TargetModeSubclass *>(getFixupInfoTable().at(K).get());
-}
+  template <EdgeKind_aarch32 K>
+  static std::unique_ptr<FixupInfoBase> initEntry() {
+    auto Entry = std::make_unique<FixupInfo<K>>();
+    if constexpr (hasOpcode<K>(0)) {
+      static_assert(isArm<K>() != isThumb<K>(), "Classes are mutually exclusive");
+      if constexpr (isArm<K>())
+        Entry->checkOpcode = checkOpcodeArm<K>;
+      if constexpr (isThumb<K>())
+        Entry->checkOpcode = checkOpcodeThumb<K>;
+    }
+    return Entry;
+  }
 
-static bool checkOpcode(const ArmRelocation &R, Edge::Kind Kind) {
+private:
+  std::array<std::unique_ptr<FixupInfoBase>, Items> Data;
+};
+
+} // namespace
+
+ManagedStatic<FixupInfoTable> DynFixupInfos;
+
+static Error
+checkOpcode(LinkGraph &G, const ArmRelocation &R, Edge::Kind Kind) {
   assert(Kind >= FirstArmRelocation && Kind <= LastArmRelocation &&
-         "Edge kind is no Arm relocation");
-  const FixupInfoArm *const Info = getDynFixupInfo<FixupInfoArm>(Kind);
-  if (LLVM_LIKELY(Info) && LLVM_LIKELY(Info->checkOpcode))
-    return Info->checkOpcode(R.Wd);
-  LLVM_DEBUG(dbgs() << "Can not perform Opcode check for aarch32 edge kind "
-                    << getEdgeKindName(Kind));
-  return true;
+         "Edge kind must be Arm relocation");
+  const FixupInfoBase *Entry = DynFixupInfos->getEntry(Kind);
+  const FixupInfoArm &Info = *static_cast<const FixupInfoArm *>(Entry);
+  assert(Info.checkOpcode && "Opcode check is mandatory for Arm edges");
+  if (!Info.checkOpcode(R.Wd))
+    return makeUnexpectedOpcodeError(G, R, Kind);
+
+  return Error::success();
 }
 
-static bool checkOpcode(const ThumbRelocation &R, Edge::Kind Kind) {
+static Error
+checkOpcode(LinkGraph &G, const ThumbRelocation &R, Edge::Kind Kind) {
   assert(Kind >= FirstThumbRelocation && Kind <= LastThumbRelocation &&
-         "Edge kind is no Thumb relocation");
-  const FixupInfoThumb *const Info = getDynFixupInfo<FixupInfoThumb>(Kind);
-  if (LLVM_LIKELY(Info) && LLVM_LIKELY(Info->checkOpcode))
-    return Info->checkOpcode(R.Hi, R.Lo);
-  LLVM_DEBUG(dbgs() << "Can not perform Opcode check for aarch32 edge kind "
-                    << getEdgeKindName(Kind));
-  return true;
+         "Edge kind must be Thumb relocation");
+  const FixupInfoBase *Entry = DynFixupInfos->getEntry(Kind);
+  const FixupInfoThumb &Info = *static_cast<const FixupInfoThumb *>(Entry);
+  assert(Info.checkOpcode && "Opcode check is mandatory for Thumb edges");
+  if (!Info.checkOpcode(R.Hi, R.Lo))
+    return makeUnexpectedOpcodeError(G, R, Kind);
+
+  return Error::success();
+}
+
+const FixupInfoBase *FixupInfoBase::getDynFixupInfo(Edge::Kind K) {
+  return DynFixupInfos->getEntry(K);
 }
 
 template <EdgeKind_aarch32 Kind>
@@ -408,8 +416,8 @@ Expected<int64_t> readAddendData(LinkGraph &G, Block &B, Edge::OffsetT Offset,
 Expected<int64_t> readAddendArm(LinkGraph &G, Block &B, Edge::OffsetT Offset,
                                 Edge::Kind Kind) {
   ArmRelocation R(B.getContent().data() + Offset);
-  if (!checkOpcode(R, Kind))
-    return makeUnexpectedOpcodeError(G, R, Kind);
+  if (Error Err = checkOpcode(G, R, Kind))
+    return std::move(Err);
 
   switch (Kind) {
   case Arm_Call:
@@ -431,8 +439,8 @@ Expected<int64_t> readAddendArm(LinkGraph &G, Block &B, Edge::OffsetT Offset,
 Expected<int64_t> readAddendThumb(LinkGraph &G, Block &B, Edge::OffsetT Offset,
                                   Edge::Kind Kind, const ArmConfig &ArmCfg) {
   ThumbRelocation R(B.getContent().data() + Offset);
-  if (!checkOpcode(R, Kind))
-    return makeUnexpectedOpcodeError(G, R, Kind);
+  if (Error Err = checkOpcode(G, R, Kind))
+    return std::move(Err);
 
   switch (Kind) {
   case Thumb_Call:
@@ -509,8 +517,8 @@ Error applyFixupData(LinkGraph &G, Block &B, const Edge &E) {
 Error applyFixupArm(LinkGraph &G, Block &B, const Edge &E) {
   WritableArmRelocation R(B.getAlreadyMutableContent().data() + E.getOffset());
   Edge::Kind Kind = E.getKind();
-  if (!checkOpcode(R, Kind))
-    return makeUnexpectedOpcodeError(G, R, Kind);
+  if (Error Err = checkOpcode(G, R, Kind))
+    return Err;
 
   uint64_t FixupAddress = (B.getAddress() + E.getOffset()).getValue();
   int64_t Addend = E.getAddend();
@@ -585,8 +593,8 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E,
   WritableThumbRelocation R(B.getAlreadyMutableContent().data() +
                             E.getOffset());
   Edge::Kind Kind = E.getKind();
-  if (!checkOpcode(R, Kind))
-    return makeUnexpectedOpcodeError(G, R, Kind);
+  if (Error Err = checkOpcode(G, R, Kind))
+    return Err;
 
   uint64_t FixupAddress = (B.getAddress() + E.getOffset()).getValue();
   int64_t Addend = E.getAddend();
diff --git a/llvm/unittests/ExecutionEngine/JITLink/AArch32Tests.cpp b/llvm/unittests/ExecutionEngine/JITLink/AArch32Tests.cpp
index dcc8d3b237ff318..5f44e4d93036259 100644
--- a/llvm/unittests/ExecutionEngine/JITLink/AArch32Tests.cpp
+++ b/llvm/unittests/ExecutionEngine/JITLink/AArch32Tests.cpp
@@ -67,6 +67,29 @@ TEST(AArch32_ELF, EdgeKinds) {
   }
 }
 
+TEST(AArch32_ELF, DynFixupInfos) {
+  // We can do an opcode check for all Arm edges
+  for (Edge::Kind K = FirstArmRelocation; K < LastArmRelocation; K += 1) {
+    const auto *Info = FixupInfoBase::getDynFixupInfo(K);
+    EXPECT_NE(Info, nullptr);
+    const auto *InfoArm = static_cast<const FixupInfoArm *>(Info);
+    EXPECT_NE(InfoArm->checkOpcode, nullptr);
+    EXPECT_FALSE(InfoArm->checkOpcode(0x00000000));
+  }
+  // We can do an opcode check for all Thumb edges
+  for (Edge::Kind K = FirstThumbRelocation; K < LastThumbRelocation; K += 1) {
+    const auto *Info = FixupInfoBase::getDynFixupInfo(K);
+    EXPECT_NE(Info, nullptr);
+    const auto *InfoThumb = static_cast<const FixupInfoThumb *>(Info);
+    EXPECT_NE(InfoThumb->checkOpcode, nullptr);
+    EXPECT_FALSE(InfoThumb->checkOpcode(0x0000, 0x0000));
+  }
+  // We cannot do it for Data and generic edges
+  EXPECT_EQ(FixupInfoBase::getDynFixupInfo(FirstDataRelocation), nullptr);
+  EXPECT_EQ(FixupInfoBase::getDynFixupInfo(Edge::GenericEdgeKind::Invalid),
+            nullptr);
+}
+
 namespace llvm {
 namespace jitlink {
 namespace aarch32 {



More information about the llvm-commits mailing list