[llvm] [JITLink][AArch32] Cleanup and refactors (NFC) (PR #78362)

Stefan Gränitz via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 16 15:29:00 PST 2024


https://github.com/weliveindetail created https://github.com/llvm/llvm-project/pull/78362

None

>From 1ec1201008d5f7e9a3ffe52dbadd3e0e5bca8c37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Mon, 8 Jan 2024 15:05:08 +0100
Subject: [PATCH 1/5] [JITLink][AArch32] In warning output add decimal value
 for CPUArch and missing newline

---
 llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
index f346cfb2a93112..d7869377a8baaa 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
@@ -146,7 +146,7 @@ inline ArmConfig getArmConfigForCPUArch(ARMBuildAttrs::CPUArch CPUArch) {
   default:
     DEBUG_WITH_TYPE("jitlink", {
       dbgs() << "  Warning: ARM config not defined for CPU architecture "
-             << getCPUArchName(CPUArch);
+             << getCPUArchName(CPUArch) << " (" << CPUArch << ")\n";
     });
     break;
   }

>From c22bf411cc8369026150c77f036a73da1e41b633 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 10 Jan 2024 17:07:28 +0100
Subject: [PATCH 2/5] [JITLink][AArch32] Streamline file-names of tests (NFC)

All other backends use the full term "relocations". Also, sorting by type (relocations/stubs/etc.) before CPU states (arm/thumb/other) makes it easier to filter in LIT.
---
 .../AArch32/{ELF_static_arm_reloc.s => ELF_relocations_arm.s}     | 0
 .../AArch32/{ELF_static_data_reloc.s => ELF_relocations_data.s}   | 0
 .../AArch32/{ELF_static_thumb_reloc.s => ELF_relocations_thumb.s} | 0
 .../JITLink/AArch32/{ELF_thumb_stubs.s => ELF_stubs_thumb.s}      | 0
 4 files changed, 0 insertions(+), 0 deletions(-)
 rename llvm/test/ExecutionEngine/JITLink/AArch32/{ELF_static_arm_reloc.s => ELF_relocations_arm.s} (100%)
 rename llvm/test/ExecutionEngine/JITLink/AArch32/{ELF_static_data_reloc.s => ELF_relocations_data.s} (100%)
 rename llvm/test/ExecutionEngine/JITLink/AArch32/{ELF_static_thumb_reloc.s => ELF_relocations_thumb.s} (100%)
 rename llvm/test/ExecutionEngine/JITLink/AArch32/{ELF_thumb_stubs.s => ELF_stubs_thumb.s} (100%)

diff --git a/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_arm_reloc.s b/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_relocations_arm.s
similarity index 100%
rename from llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_arm_reloc.s
rename to llvm/test/ExecutionEngine/JITLink/AArch32/ELF_relocations_arm.s
diff --git a/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_data_reloc.s b/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_relocations_data.s
similarity index 100%
rename from llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_data_reloc.s
rename to llvm/test/ExecutionEngine/JITLink/AArch32/ELF_relocations_data.s
diff --git a/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_thumb_reloc.s b/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_relocations_thumb.s
similarity index 100%
rename from llvm/test/ExecutionEngine/JITLink/AArch32/ELF_static_thumb_reloc.s
rename to llvm/test/ExecutionEngine/JITLink/AArch32/ELF_relocations_thumb.s
diff --git a/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_thumb_stubs.s b/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_stubs_thumb.s
similarity index 100%
rename from llvm/test/ExecutionEngine/JITLink/AArch32/ELF_thumb_stubs.s
rename to llvm/test/ExecutionEngine/JITLink/AArch32/ELF_stubs_thumb.s

>From 4db9789b202d5557adeba6d5494ae0b8049a9fc6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Sat, 13 Jan 2024 23:29:33 +0100
Subject: [PATCH 3/5] [JITLink][AArch32] Fix typos in Thumb stubs test (NFC)

---
 llvm/test/ExecutionEngine/JITLink/AArch32/ELF_stubs_thumb.s | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_stubs_thumb.s b/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_stubs_thumb.s
index bd95c375279246..f6156628ce2a9f 100644
--- a/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_stubs_thumb.s
+++ b/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_stubs_thumb.s
@@ -1,6 +1,6 @@
 # RUN: rm -rf %t && mkdir -p %t
 # RUN: llvm-mc -triple=thumbv7-linux-gnueabi -arm-add-build-attributes \
-# RUN:         -filetype=obj -filetype=obj -o %t/elf_stubs.o %s
+# RUN:         -filetype=obj -o %t/elf_stubs.o %s
 # RUN: llvm-jitlink -noexec -slab-address 0x76ff0000 \
 # RUN:              -slab-allocate 10Kb -slab-page-size 4096 \
 # RUN:              -abs external_func=0x76bbe880 \
@@ -26,7 +26,7 @@ test_external_call:
 	.size test_external_call, .-test_external_call
 
 	.globl  test_external_jump
-	.type	test_external_call,%function
+	.type	test_external_jump,%function
 	.p2align	1
 	.code	16
 	.thumb_func

>From fc43344f992ea33325e9409031baed5836a57a15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Sat, 13 Jan 2024 23:45:15 +0100
Subject: [PATCH 4/5] [JITLink][AArch32] Rename stubs flavor Thumbv7 to v7
 (NFC)

---
 llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h | 12 ++++++------
 llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp    |  8 ++++----
 llvm/lib/ExecutionEngine/JITLink/aarch32.cpp        |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
index d7869377a8baaa..53c9ef2fdf395e 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
@@ -123,15 +123,15 @@ const char *getEdgeKindName(Edge::Kind K);
 ///
 /// Stubs are often called "veneers" in the official docs and online.
 ///
-enum StubsFlavor {
+enum class StubsFlavor {
   Unsupported = 0,
-  Thumbv7,
+  v7,
 };
 
 /// JITLink sub-arch configuration for Arm CPU models
 struct ArmConfig {
   bool J1J2BranchEncoding = false;
-  StubsFlavor Stubs = Unsupported;
+  StubsFlavor Stubs = StubsFlavor::Unsupported;
 };
 
 /// Obtain the sub-arch configuration for a given Arm CPU model.
@@ -141,7 +141,7 @@ inline ArmConfig getArmConfigForCPUArch(ARMBuildAttrs::CPUArch CPUArch) {
   case ARMBuildAttrs::v7:
   case ARMBuildAttrs::v8_A:
     ArmCfg.J1J2BranchEncoding = true;
-    ArmCfg.Stubs = Thumbv7;
+    ArmCfg.Stubs = StubsFlavor::v7;
     break;
   default:
     DEBUG_WITH_TYPE("jitlink", {
@@ -380,9 +380,9 @@ class StubsManager : public TableManager<StubsManager<Flavor>> {
 
 /// Create a branch range extension stub with Thumb encoding for v7 CPUs.
 template <>
-Symbol &StubsManager<Thumbv7>::createEntry(LinkGraph &G, Symbol &Target);
+Symbol &StubsManager<StubsFlavor::v7>::createEntry(LinkGraph &G, Symbol &Target);
 
-template <> inline StringRef StubsManager<Thumbv7>::getSectionName() {
+template <> inline StringRef StubsManager<StubsFlavor::v7>::getSectionName() {
   return "__llvm_jitlink_aarch32_STUBS_Thumbv7";
 }
 
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
index 132989fcbce021..3c596a414363e4 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
@@ -257,7 +257,7 @@ createLinkGraphFromELFObject_aarch32(MemoryBufferRef ObjectBuffer) {
   case v7:
   case v8_A:
     ArmCfg = aarch32::getArmConfigForCPUArch(Arch);
-    assert(ArmCfg.Stubs != aarch32::Unsupported &&
+    assert(ArmCfg.Stubs != aarch32::StubsFlavor::Unsupported &&
            "Provide a config for each supported CPU");
     break;
   default:
@@ -309,11 +309,11 @@ void link_ELF_aarch32(std::unique_ptr<LinkGraph> G,
       PassCfg.PrePrunePasses.push_back(markAllSymbolsLive);
 
     switch (ArmCfg.Stubs) {
-    case aarch32::Thumbv7:
+    case aarch32::StubsFlavor::v7:
       PassCfg.PostPrunePasses.push_back(
-          buildTables_ELF_aarch32<aarch32::Thumbv7>);
+          buildTables_ELF_aarch32<aarch32::StubsFlavor::v7>);
       break;
-    case aarch32::Unsupported:
+    case aarch32::StubsFlavor::Unsupported:
       llvm_unreachable("Check before building graph");
     }
   }
diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
index 671ee1a8125252..4e75bb4882a28a 100644
--- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
@@ -685,7 +685,7 @@ const uint8_t Thumbv7ABS[] = {
 };
 
 template <>
-Symbol &StubsManager<Thumbv7>::createEntry(LinkGraph &G, Symbol &Target) {
+Symbol &StubsManager<StubsFlavor::v7>::createEntry(LinkGraph &G, Symbol &Target) {
   constexpr uint64_t Alignment = 4;
   Block &B = addStub(G, Thumbv7ABS, Alignment);
   LLVM_DEBUG({

>From 7783accf44cabf49f4eb534176f285a43a811576 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Tue, 16 Jan 2024 23:56:34 +0100
Subject: [PATCH 5/5] [JITLink][AArch32] Refactor StubsManager (NFC)

---
 .../llvm/ExecutionEngine/JITLink/aarch32.h    | 21 +++++++------------
 .../ExecutionEngine/JITLink/ELF_aarch32.cpp   |  8 +++----
 llvm/lib/ExecutionEngine/JITLink/aarch32.cpp  |  3 +--
 3 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
index 53c9ef2fdf395e..7765208b5e3dfe 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h
@@ -318,7 +318,7 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
   llvm_unreachable("Relocation must be of class Data, Arm or Thumb");
 }
 
-/// Stubs builder for a specific StubsFlavor
+/// Stubs builder for v7 emits non-position-independent Thumb stubs.
 ///
 /// Right now we only have one default stub kind, but we want to extend this
 /// and allow creation of specific kinds in the future (e.g. branch range
@@ -326,13 +326,14 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E,
 ///
 /// Let's keep it simple for the moment and not wire this through a GOT.
 ///
-template <StubsFlavor Flavor>
-class StubsManager : public TableManager<StubsManager<Flavor>> {
+class StubsManager_v7 : public TableManager<StubsManager_v7> {
 public:
-  StubsManager() = default;
+  StubsManager_v7() = default;
 
   /// Name of the object file section that will contain all our stubs.
-  static StringRef getSectionName();
+  static StringRef getSectionName() {
+    return "__llvm_jitlink_aarch32_STUBS_Thumbv7";
+  }
 
   /// Implements link-graph traversal via visitExistingEdges().
   bool visitEdge(LinkGraph &G, Block *B, Edge &E) {
@@ -354,7 +355,7 @@ class StubsManager : public TableManager<StubsManager<Flavor>> {
     return false;
   }
 
-  /// Create a branch range extension stub for the class's flavor.
+  /// Create a branch range extension stub with Thumb encoding for v7 CPUs.
   Symbol &createEntry(LinkGraph &G, Symbol &Target);
 
 private:
@@ -378,14 +379,6 @@ class StubsManager : public TableManager<StubsManager<Flavor>> {
   Section *StubsSection = nullptr;
 };
 
-/// Create a branch range extension stub with Thumb encoding for v7 CPUs.
-template <>
-Symbol &StubsManager<StubsFlavor::v7>::createEntry(LinkGraph &G, Symbol &Target);
-
-template <> inline StringRef StubsManager<StubsFlavor::v7>::getSectionName() {
-  return "__llvm_jitlink_aarch32_STUBS_Thumbv7";
-}
-
 } // namespace aarch32
 } // namespace jitlink
 } // namespace llvm
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
index 3c596a414363e4..b862a7ba2acc92 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp
@@ -216,12 +216,12 @@ class ELFLinkGraphBuilder_aarch32
         ArmCfg(std::move(ArmCfg)) {}
 };
 
-template <aarch32::StubsFlavor Flavor>
+template <typename StubsManagerType>
 Error buildTables_ELF_aarch32(LinkGraph &G) {
   LLVM_DEBUG(dbgs() << "Visiting edges in graph:\n");
 
-  aarch32::StubsManager<Flavor> PLT;
-  visitExistingEdges(G, PLT);
+  StubsManagerType StubsManager;
+  visitExistingEdges(G, StubsManager);
   return Error::success();
 }
 
@@ -311,7 +311,7 @@ void link_ELF_aarch32(std::unique_ptr<LinkGraph> G,
     switch (ArmCfg.Stubs) {
     case aarch32::StubsFlavor::v7:
       PassCfg.PostPrunePasses.push_back(
-          buildTables_ELF_aarch32<aarch32::StubsFlavor::v7>);
+          buildTables_ELF_aarch32<aarch32::StubsManager_v7>);
       break;
     case aarch32::StubsFlavor::Unsupported:
       llvm_unreachable("Check before building graph");
diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
index 4e75bb4882a28a..8153c97deff628 100644
--- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp
@@ -684,8 +684,7 @@ const uint8_t Thumbv7ABS[] = {
     0x60, 0x47              // bx   r12
 };
 
-template <>
-Symbol &StubsManager<StubsFlavor::v7>::createEntry(LinkGraph &G, Symbol &Target) {
+Symbol &StubsManager_v7::createEntry(LinkGraph &G, Symbol &Target) {
   constexpr uint64_t Alignment = 4;
   Block &B = addStub(G, Thumbv7ABS, Alignment);
   LLVM_DEBUG({



More information about the llvm-commits mailing list