<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 24, 2014 at 10:00 AM, Renato Golin <span dir="ltr"><<a href="mailto:renato.golin@linaro.org" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=renato.golin@linaro.org&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">renato.golin@linaro.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: rengolin<br>
Date: Wed Sep 24 12:00:42 2014<br>
New Revision: 218388<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=218388&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=218388&view=rev</a><br>
Log:<br>
Refactor the RelocVisitor::visit method<br>
<br>
This change replaces the brittle if/else chain of string comparisons<br>
with a switch statement on the detected target triple, removing the<br>
need for testing arbitrary architecture names returned from<br>
getFileFormatName, whose primary purpose seems to be for display<br>
(user-interface) purposes. The visitor now takes a reference to the<br>
object file, rather than its arbitrary file format name to figure out<br>
whether the file is a 32 or 64-bit object file and what the detected<br>
target triple is.<br>
<br>
A set of tests have been added to help show that the refactoring processes<br>
relocations for the same targets as the original code.<br>
<br>
Patch by Charlie Turner.<br>
<br>
Added:<br>
    llvm/trunk/test/DebugInfo/processes-relocations.ll<br>
Modified:<br>
    llvm/trunk/include/llvm/Object/ELFObjectFile.h<br>
    llvm/trunk/include/llvm/Object/RelocVisitor.h<br>
    llvm/trunk/lib/DebugInfo/DWARFContext.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Object/ELFObjectFile.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/ELFObjectFile.h?rev=218388&r1=218387&r2=218388&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/ELFObjectFile.h?rev=218388&r1=218387&r2=218388&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Object/ELFObjectFile.h (original)<br>
+++ llvm/trunk/include/llvm/Object/ELFObjectFile.h Wed Sep 24 12:00:42 2014<br>
@@ -924,6 +924,8 @@ unsigned ELFObjectFile<ELFT>::getArch()<br>
     default:<br>
       report_fatal_error("Invalid ELFCLASS!");<br>
     }<br>
+  case ELF::EM_PPC:<br>
+    return Triple::ppc;<br>
   case ELF::EM_PPC64:<br>
     return IsLittleEndian ? Triple::ppc64le : Triple::ppc64;<br>
   case ELF::EM_S390:<br>
<br>
Modified: llvm/trunk/include/llvm/Object/RelocVisitor.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/RelocVisitor.h?rev=218388&r1=218387&r2=218388&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/RelocVisitor.h?rev=218388&r1=218387&r2=218388&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Object/RelocVisitor.h (original)<br>
+++ llvm/trunk/include/llvm/Object/RelocVisitor.h Wed Sep 24 12:00:42 2014<br>
@@ -40,16 +40,18 @@ struct RelocToApply {<br>
 /// @brief Base class for object file relocation visitors.<br>
 class RelocVisitor {<br>
 public:<br>
-  explicit RelocVisitor(StringRef FileFormat)<br>
-    : FileFormat(FileFormat), HasError(false) {}<br>
+  explicit RelocVisitor(ObjectFile &Obj)<br>
+    : ObjToVisit(Obj), HasError(false) {}<br>
<br>
   // TODO: Should handle multiple applied relocations via either passing in the<br>
   // previously computed value or just count paired relocations as a single<br>
   // visit.<br>
   RelocToApply visit(uint32_t RelocType, RelocationRef R, uint64_t SecAddr = 0,<br>
                      uint64_t Value = 0) {<br>
-    if (FileFormat == "ELF64-x86-64") {<br>
-      switch (RelocType) {<br>
+    if (ObjToVisit.getBytesInAddress() == 8) { // 64-bit object file<br>
+      switch (ObjToVisit.getArch()) {<br>
+      case Triple::x86_64:<br>
+        switch (RelocType) {<br>
         case llvm::ELF::R_X86_64_NONE:<br>
           return visitELF_X86_64_NONE(R);<br>
         case llvm::ELF::R_X86_64_64:<br>
@@ -63,113 +65,129 @@ public:<br>
         default:<br>
           HasError = true;<br>
           return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF32-i386") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_386_NONE:<br>
-        return visitELF_386_NONE(R);<br>
-      case llvm::ELF::R_386_32:<br>
-        return visitELF_386_32(R, Value);<br>
-      case llvm::ELF::R_386_PC32:<br>
-        return visitELF_386_PC32(R, Value, SecAddr);<br>
-      default:<br>
-        HasError = true;<br>
-        return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF64-ppc64") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_PPC64_ADDR32:<br>
-        return visitELF_PPC64_ADDR32(R, Value);<br>
-      case llvm::ELF::R_PPC64_ADDR64:<br>
-        return visitELF_PPC64_ADDR64(R, Value);<br>
-      default:<br>
-        HasError = true;<br>
-        return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF32-ppc") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_PPC_ADDR32:<br>
-        return visitELF_PPC_ADDR32(R, Value);<br>
-      default:<br>
-        HasError = true;<br>
-        return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF32-mips") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_MIPS_32:<br>
-        return visitELF_MIPS_32(R, Value);<br>
-      default:<br>
-        HasError = true;<br>
-        return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF64-mips") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_MIPS_32:<br>
-        return visitELF_MIPS_32(R, Value);<br>
-      case llvm::ELF::R_MIPS_64:<br>
-        return visitELF_MIPS_64(R, Value);<br>
-      default:<br>
-        HasError = true;<br>
-        return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF64-aarch64") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_AARCH64_ABS32:<br>
-        return visitELF_AARCH64_ABS32(R, Value);<br>
-      case llvm::ELF::R_AARCH64_ABS64:<br>
-        return visitELF_AARCH64_ABS64(R, Value);<br>
-      default:<br>
-        HasError = true;<br>
-        return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF64-s390") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_390_32:<br>
-        return visitELF_390_32(R, Value);<br>
-      case llvm::ELF::R_390_64:<br>
-        return visitELF_390_64(R, Value);<br>
-      default:<br>
-        HasError = true;<br>
-        return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF32-sparc") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_SPARC_32:<br>
-      case llvm::ELF::R_SPARC_UA32:<br>
-        return visitELF_SPARC_32(R, Value);<br>
-      default:<br>
-        HasError = true;<br>
-        return RelocToApply();<br>
-      }<br>
-    } else if (FileFormat == "ELF64-sparc") {<br>
-      switch (RelocType) {<br>
-      case llvm::ELF::R_SPARC_32:<br>
-      case llvm::ELF::R_SPARC_UA32:<br>
-        return visitELF_SPARCV9_32(R, Value);<br>
-      case llvm::ELF::R_SPARC_64:<br>
-      case llvm::ELF::R_SPARC_UA64:<br>
-        return visitELF_SPARCV9_64(R, Value);<br>
+        }<br>
+      case Triple::aarch64:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_AARCH64_ABS32:<br>
+          return visitELF_AARCH64_ABS32(R, Value);<br>
+        case llvm::ELF::R_AARCH64_ABS64:<br>
+          return visitELF_AARCH64_ABS64(R, Value);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
+      case Triple::mips64el:<br>
+      case Triple::mips64:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_MIPS_32:<br>
+          return visitELF_MIPS_32(R, Value);<br>
+        case llvm::ELF::R_MIPS_64:<br>
+          return visitELF_MIPS_64(R, Value);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
+      case Triple::ppc64le:<br>
+      case Triple::ppc64:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_PPC64_ADDR32:<br>
+          return visitELF_PPC64_ADDR32(R, Value);<br>
+        case llvm::ELF::R_PPC64_ADDR64:<br>
+          return visitELF_PPC64_ADDR64(R, Value);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
+      case Triple::systemz:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_390_32:<br>
+          return visitELF_390_32(R, Value);<br>
+        case llvm::ELF::R_390_64:<br>
+          return visitELF_390_64(R, Value);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
+      case Triple::sparcv9:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_SPARC_32:<br>
+        case llvm::ELF::R_SPARC_UA32:<br>
+          return visitELF_SPARCV9_32(R, Value);<br>
+        case llvm::ELF::R_SPARC_64:<br>
+        case llvm::ELF::R_SPARC_UA64:<br>
+          return visitELF_SPARCV9_64(R, Value);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
       default:<br>
         HasError = true;<br>
         return RelocToApply();<br>
       }<br>
-    } else if (FileFormat == "ELF32-arm") {<br>
-      switch (RelocType) {<br>
+    } else if (ObjToVisit.getBytesInAddress() == 4) { // 32-bit object file<br>
+      switch (ObjToVisit.getArch()) {<br>
+      case Triple::x86:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_386_NONE:<br>
+          return visitELF_386_NONE(R);<br>
+        case llvm::ELF::R_386_32:<br>
+          return visitELF_386_32(R, Value);<br>
+        case llvm::ELF::R_386_PC32:<br>
+          return visitELF_386_PC32(R, Value, SecAddr);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
+      case Triple::ppc:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_PPC_ADDR32:<br>
+          return visitELF_PPC_ADDR32(R, Value);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
+      case Triple::arm:<br>
+      case Triple::armeb:<br>
+        switch (RelocType) {<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        case llvm::ELF::R_ARM_ABS32:<br>
+          return visitELF_ARM_ABS32(R, Value);<br>
+        }<br>
+      case Triple::hexagon:<br>
+        llvm_unreachable("Unimplemented");<br>
+      case Triple::mipsel:<br>
+      case Triple::mips:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_MIPS_32:<br>
+          return visitELF_MIPS_32(R, Value);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
+      case Triple::sparc:<br>
+        switch (RelocType) {<br>
+        case llvm::ELF::R_SPARC_32:<br>
+        case llvm::ELF::R_SPARC_UA32:<br>
+          return visitELF_SPARC_32(R, Value);<br>
+        default:<br>
+          HasError = true;<br>
+          return RelocToApply();<br>
+        }<br>
       default:<br>
         HasError = true;<br>
         return RelocToApply();<br>
-      case llvm::ELF::R_ARM_ABS32:<br>
-        return visitELF_ARM_ABS32(R, Value);<br>
       }<br>
+    } else {<br>
+      report_fatal_error("Invalid word size in object file");<br>
     }<br>
-    HasError = true;<br>
-    return RelocToApply();<br>
   }<br>
<br>
   bool error() { return HasError; }<br>
<br>
 private:<br>
-  StringRef FileFormat;<br>
+  ObjectFile &ObjToVisit;<br>
   bool HasError;<br>
<br>
   int64_t getAddend32LE(RelocationRef R) {<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/DWARFContext.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.cpp?rev=218388&r1=218387&r2=218388&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFContext.cpp?rev=218388&r1=218387&r2=218388&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/DWARFContext.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/DWARFContext.cpp Wed Sep 24 12:00:42 2014<br>
@@ -667,7 +667,7 @@ DWARFContextInMemory::DWARFContextInMemo<br>
           Sym->getAddress(SymAddr);<br>
         }<br>
<br>
-        object::RelocVisitor V(Obj.getFileFormatName());<br>
+        object::RelocVisitor V(Obj);<br>
         // The section address is always 0 for debug sections.<br>
         object::RelocToApply R(V.visit(Type, Reloc, 0, SymAddr));<br>
         if (V.error()) {<br>
<br>
Added: llvm/trunk/test/DebugInfo/processes-relocations.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/processes-relocations.ll?rev=218388&view=auto" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/processes-relocations.ll?rev=218388&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/processes-relocations.ll (added)<br>
+++ llvm/trunk/test/DebugInfo/processes-relocations.ll Wed Sep 24 12:00:42 2014<br>
@@ -0,0 +1,26 @@<br>
+; Make sure that for each supported architecture in RelocVisitor::visit,<br>
+; the visitor does compute the relocation.<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple x86_64-none-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple i386-none-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple powerpc64-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple powerpc-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple mips-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple mips64-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple arm-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple aarch64-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple s390x-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple sparc-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br>
+; RUN: llc -filetype=obj -O0 < %s -mtriple sparcv9-unknown-linux-eabi | llvm-dwarfdump - 2>&1 | FileCheck %s<br></blockquote><div><br></div><div>FYI, this test implicitly requires all of these targets to be built in to llc or else the test will fail... so if someone ran cmake with e.g. -DLLVM_TARGETS_TO_BUILD="X86;PowerPC" as described in <a href="http://llvm.org/docs/CMake.html">http://llvm.org/docs/CMake.html</a> they would encounter spurious test failures.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+<br>
+; CHECK-NOT: failed to compute relocation<br>
+<br>
+!<a href="http://llvm.dbg.cu" target="_blank" class="cremed">llvm.dbg.cu</a> = !{!0}<br>
+!llvm.module.flags = !{!3, !4}<br>
+!llvm.ident = !{!5}<br>
+<br>
+!0 = metadata !{i32 786449, metadata !1, i32 12, metadata !"clang version 3.6.0 ", i1 false, metadata !"", i32 0, metadata !2, metadata !2, metadata !2, metadata !2, metadata !2, metadata !"", i32 1} ; [ DW_TAG_compile_unit ] [/a/empty.c] [DW_LANG_C99]<br>
+!1 = metadata !{metadata !"empty.c", metadata !"/a"}<br>
+!2 = metadata !{}<br>
+!3 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}<br>
+!4 = metadata !{i32 2, metadata !"Debug Info Version", i32 1}<br>
+!5 = metadata !{metadata !"clang version 3.6.0 "}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=llvm-commits@cs.uiuc.edu&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>