[Object][COFF][NFC] Introduce getMachineArchType helper. #87370

cjacek · 2024-04-02T17:03:02Z

As mentioned in #87240, it's a common pattern that we have a machine type, but we don't care about which ARM64* platform we're dealing with. We already have isAnyArm64 for that, but it does not fit cases where we use a switch statement. With this helper, it's easy to simplify such cases by using Triple::ArchType instead.

This PR introduces the helper and replaces existing use cases in llvm/. I also created a branch that uses it in LLD: https://github.com/cjacek/llvm-project/commits/machine-arch-wip

llvmbot · 2024-04-02T17:03:31Z

@llvm/pr-subscribers-llvm-binary-utilities

Author: Jacek Caban (cjacek)

Changes

As mentioned in #87240, it's a common pattern that we have a machine type, but we don't care about which ARM64* platform we're dealing with. We already have isAnyArm64 for that, but it does not fit cases where we use a switch statement. With this helper, it's easy to simplify such cases by using Triple::ArchType instead.

This PR introduces the helper and replaces existing use cases in llvm/. I also created a branch that uses it in LLD: https://github.com/cjacek/llvm-project/commits/machine-arch-wip

Full diff: https://github.com/llvm/llvm-project/pull/87370.diff

3 Files Affected:

(modified) llvm/include/llvm/Object/WindowsMachineFlag.h (+19)
(modified) llvm/lib/Object/COFFObjectFile.cpp (+12-30)
(modified) llvm/lib/Object/WindowsResource.cpp (+6-7)

diff --git a/llvm/include/llvm/Object/WindowsMachineFlag.h b/llvm/include/llvm/Object/WindowsMachineFlag.h
index 05b8f0d52d3fdc..436a6827a6c8fe 100644
--- a/llvm/include/llvm/Object/WindowsMachineFlag.h
+++ b/llvm/include/llvm/Object/WindowsMachineFlag.h
@@ -13,6 +13,9 @@
 #ifndef LLVM_OBJECT_WINDOWSMACHINEFLAG_H
 #define LLVM_OBJECT_WINDOWSMACHINEFLAG_H
 
+#include "llvm/BinaryFormat/COFF.h"
+#include "llvm/TargetParser/Triple.h"
+
 namespace llvm {
 
 class StringRef;
@@ -28,6 +31,22 @@ StringRef machineToStr(COFF::MachineTypes MT);
 // Only returns ARMNT, ARM64, AMD64, I386, or IMAGE_FILE_MACHINE_UNKNOWN.
 COFF::MachineTypes getMachineType(StringRef S);
 
+template <typename T> Triple::ArchType getMachineArchType(T machine) {
+  switch (machine) {
+  case COFF::IMAGE_FILE_MACHINE_I386:
+    return llvm::Triple::ArchType::x86;
+  case COFF::IMAGE_FILE_MACHINE_AMD64:
+    return llvm::Triple::ArchType::x86_64;
+  case COFF::IMAGE_FILE_MACHINE_ARMNT:
+    return llvm::Triple::ArchType::thumb;
+  case COFF::IMAGE_FILE_MACHINE_ARM64:
+  case COFF::IMAGE_FILE_MACHINE_ARM64EC:
+  case COFF::IMAGE_FILE_MACHINE_ARM64X:
+    return llvm::Triple::ArchType::aarch64;
+  default:
+    return llvm::Triple::ArchType::UnknownArch;
+  }
 }
 
+} // namespace llvm
 #endif
diff --git a/llvm/lib/Object/COFFObjectFile.cpp b/llvm/lib/Object/COFFObjectFile.cpp
index 8700912614db8d..18506f39f6b57b 100644
--- a/llvm/lib/Object/COFFObjectFile.cpp
+++ b/llvm/lib/Object/COFFObjectFile.cpp
@@ -14,18 +14,17 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/iterator_range.h"
-#include "llvm/BinaryFormat/COFF.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Object/WindowsMachineFlag.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBufferRef.h"
-#include "llvm/TargetParser/Triple.h"
 #include <algorithm>
 #include <cassert>
 #include <cinttypes>
@@ -1072,20 +1071,7 @@ StringRef COFFObjectFile::getFileFormatName() const {
 }
 
 Triple::ArchType COFFObjectFile::getArch() const {
-  switch (getMachine()) {
-  case COFF::IMAGE_FILE_MACHINE_I386:
-    return Triple::x86;
-  case COFF::IMAGE_FILE_MACHINE_AMD64:
-    return Triple::x86_64;
-  case COFF::IMAGE_FILE_MACHINE_ARMNT:
-    return Triple::thumb;
-  case COFF::IMAGE_FILE_MACHINE_ARM64:
-  case COFF::IMAGE_FILE_MACHINE_ARM64EC:
-  case COFF::IMAGE_FILE_MACHINE_ARM64X:
-    return Triple::aarch64;
-  default:
-    return Triple::UnknownArch;
-  }
+  return getMachineArchType(getMachine());
 }
 
 Expected<uint64_t> COFFObjectFile::getStartAddress() const {
@@ -1320,8 +1306,8 @@ COFFObjectFile::getRelocations(const coff_section *Sec) const {
     return #reloc_type;
 
 StringRef COFFObjectFile::getRelocationTypeName(uint16_t Type) const {
-  switch (getMachine()) {
-  case COFF::IMAGE_FILE_MACHINE_AMD64:
+  switch (getArch()) {
+  case Triple::x86_64:
     switch (Type) {
     LLVM_COFF_SWITCH_RELOC_TYPE_NAME(IMAGE_REL_AMD64_ABSOLUTE);
     LLVM_COFF_SWITCH_RELOC_TYPE_NAME(IMAGE_REL_AMD64_ADDR64);
@@ -1344,7 +1330,7 @@ StringRef COFFObjectFile::getRelocationTypeName(uint16_t Type) const {
       return "Unknown";
     }
     break;
-  case COFF::IMAGE_FILE_MACHINE_ARMNT:
+  case Triple::thumb:
     switch (Type) {
     LLVM_COFF_SWITCH_RELOC_TYPE_NAME(IMAGE_REL_ARM_ABSOLUTE);
     LLVM_COFF_SWITCH_RELOC_TYPE_NAME(IMAGE_REL_ARM_ADDR32);
@@ -1367,9 +1353,7 @@ StringRef COFFObjectFile::getRelocationTypeName(uint16_t Type) const {
       return "Unknown";
     }
     break;
-  case COFF::IMAGE_FILE_MACHINE_ARM64:
-  case COFF::IMAGE_FILE_MACHINE_ARM64EC:
-  case COFF::IMAGE_FILE_MACHINE_ARM64X:
+  case Triple::aarch64:
     switch (Type) {
     LLVM_COFF_SWITCH_RELOC_TYPE_NAME(IMAGE_REL_ARM64_ABSOLUTE);
     LLVM_COFF_SWITCH_RELOC_TYPE_NAME(IMAGE_REL_ARM64_ADDR32);
@@ -1393,7 +1377,7 @@ StringRef COFFObjectFile::getRelocationTypeName(uint16_t Type) const {
       return "Unknown";
     }
     break;
-  case COFF::IMAGE_FILE_MACHINE_I386:
+  case Triple::x86:
     switch (Type) {
     LLVM_COFF_SWITCH_RELOC_TYPE_NAME(IMAGE_REL_I386_ABSOLUTE);
     LLVM_COFF_SWITCH_RELOC_TYPE_NAME(IMAGE_REL_I386_DIR16);
@@ -1941,19 +1925,17 @@ ResourceSectionRef::getContents(const coff_resource_data_entry &Entry) {
     // the expected type.
     const coff_relocation &R = **RelocsForOffset.first;
     uint16_t RVAReloc;
-    switch (Obj->getMachine()) {
-    case COFF::IMAGE_FILE_MACHINE_I386:
+    switch (Obj->getArch()) {
+    case Triple::x86:
       RVAReloc = COFF::IMAGE_REL_I386_DIR32NB;
       break;
-    case COFF::IMAGE_FILE_MACHINE_AMD64:
+    case Triple::x86_64:
       RVAReloc = COFF::IMAGE_REL_AMD64_ADDR32NB;
       break;
-    case COFF::IMAGE_FILE_MACHINE_ARMNT:
+    case Triple::thumb:
       RVAReloc = COFF::IMAGE_REL_ARM_ADDR32NB;
       break;
-    case COFF::IMAGE_FILE_MACHINE_ARM64:
-    case COFF::IMAGE_FILE_MACHINE_ARM64EC:
-    case COFF::IMAGE_FILE_MACHINE_ARM64X:
+    case Triple::aarch64:
       RVAReloc = COFF::IMAGE_REL_ARM64_ADDR32NB;
       break;
     default:
diff --git a/llvm/lib/Object/WindowsResource.cpp b/llvm/lib/Object/WindowsResource.cpp
index 61ca49e290da18..983c8e30a9420d 100644
--- a/llvm/lib/Object/WindowsResource.cpp
+++ b/llvm/lib/Object/WindowsResource.cpp
@@ -12,6 +12,7 @@
 
 #include "llvm/Object/WindowsResource.h"
 #include "llvm/Object/COFF.h"
+#include "llvm/Object/WindowsMachineFlag.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -978,19 +979,17 @@ void WindowsResourceCOFFWriter::writeFirstSectionRelocations() {
         reinterpret_cast<coff_relocation *>(BufferStart + CurrentOffset);
     Reloc->VirtualAddress = RelocationAddresses[i];
     Reloc->SymbolTableIndex = NextSymbolIndex++;
-    switch (MachineType) {
-    case COFF::IMAGE_FILE_MACHINE_ARMNT:
+    switch (getMachineArchType(MachineType)) {
+    case Triple::thumb:
       Reloc->Type = COFF::IMAGE_REL_ARM_ADDR32NB;
       break;
-    case COFF::IMAGE_FILE_MACHINE_AMD64:
+    case Triple::x86_64:
       Reloc->Type = COFF::IMAGE_REL_AMD64_ADDR32NB;
       break;
-    case COFF::IMAGE_FILE_MACHINE_I386:
+    case Triple::x86:
       Reloc->Type = COFF::IMAGE_REL_I386_DIR32NB;
       break;
-    case COFF::IMAGE_FILE_MACHINE_ARM64:
-    case COFF::IMAGE_FILE_MACHINE_ARM64EC:
-    case COFF::IMAGE_FILE_MACHINE_ARM64X:
+    case Triple::aarch64:
       Reloc->Type = COFF::IMAGE_REL_ARM64_ADDR32NB;
       break;
     default:

MaskRay · 2024-04-03T04:14:47Z

llvm/include/llvm/Object/WindowsMachineFlag.h

 }

+} // namespace llvm
 #endif


there is normally a blank line before #endif

Fixed and pushed, thanks.

Adds support for ARM64EC, which should use the same search paths as ARM64. It's similar to #87370 and #87495. The test is based on the existing x86 test. Generally ARM64EC libraries are shipped together with native ARM64 libraries (using ECSYMBOLS section mechanism). getMachineArchType uses Triple::thumb, while the existing implementation uses Triple::arm. It's ultimately passed to MSVCPaths.cpp functions, so modify them to accept both forms.

It's similar to #87370, but for lld-link.

cjacek requested review from mstorsjo, MaskRay, dpaoliello, jh7370 and efriedma-quic April 2, 2024 17:03

llvmbot added the llvm:binary-utilities label Apr 2, 2024

cjacek mentioned this pull request Apr 2, 2024

[LLD][COFF] Add ARM64EC support to findLineTable. #87240

Merged

dpaoliello approved these changes Apr 2, 2024

View reviewed changes

efriedma-quic approved these changes Apr 2, 2024

View reviewed changes

MaskRay reviewed Apr 3, 2024

View reviewed changes

MaskRay approved these changes Apr 3, 2024

View reviewed changes

[Object][COFF][NFC] Introduce getMachineArchType helper.

2de996d

cjacek force-pushed the machine-arch branch from 4e84cc9 to 2de996d Compare April 3, 2024 11:20

cjacek merged commit 5c1544c into llvm:main Apr 3, 2024

cjacek deleted the machine-arch branch April 3, 2024 13:29

This was referenced Apr 3, 2024

[LLD][COFF][NFC] Use getMachineArchType helper. #87495

Merged

[LLD][COFF] Use getMachineArchType in LinkerDriver::getArch. #87499

Merged

cjacek added a commit that referenced this pull request Apr 4, 2024

[LLD][COFF][NFC] Use getMachineArchType helper. (#87495)

8f9903d

It's similar to #87370, but for lld-link.

cjacek mentioned this pull request Aug 28, 2024

[LLD][COFF] Add support for range extension thunks for ARM64EC targets. #106289

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Object][COFF][NFC] Introduce getMachineArchType helper. #87370

[Object][COFF][NFC] Introduce getMachineArchType helper. #87370

Uh oh!

cjacek commented Apr 2, 2024

Uh oh!

llvmbot commented Apr 2, 2024

Uh oh!

MaskRay Apr 3, 2024

Uh oh!

cjacek Apr 3, 2024

Uh oh!

Uh oh!

[Object][COFF][NFC] Introduce getMachineArchType helper. #87370

[Object][COFF][NFC] Introduce getMachineArchType helper. #87370

Uh oh!

Conversation

cjacek commented Apr 2, 2024

Uh oh!

llvmbot commented Apr 2, 2024

Uh oh!

MaskRay Apr 3, 2024

Choose a reason for hiding this comment

Uh oh!

cjacek Apr 3, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!