[LTO] Infer EKind/EMachine from Bitcode files

[davide]

Summary

This is a proposed fix for https://llvm.org/bugs/show_bug.cgi?id=28268
I hope there's an easier way to get these informations because the current code is a little bit cumbersome :(

[ruiu]

ELF/InputFiles.cpp

691 ↗

(On Diff #62049)

Instead of defining a special function for bitcode files, can you define getElfKind and getMachineType to BitcodeFile?

[davide]

ELF/InputFiles.cpp

691 ↗

(On Diff #62049)

Sorry, maybe I'm misreading but you mean defining them as member functions of class BitcodeFile similarly to what happen in class ELFFileBase?
Or are you suggesting something else?

static ELFKind getELFKind();
const llvm::object::ELFFile<ELFT> &getObj() const { return ELFObj; }
llvm::object::ELFFile<ELFT> &getObj() { return ELFObj; }

uint16_t getEMachine() const { return getObj().getHeader()->e_machine; }

[pcc]

ELF/InputFiles.cpp

707 ↗

(On Diff #62049)

Maybe use getArch() and a switch statement instead of getArchName() here?

[silvas]

ELF/InputFiles.cpp

708 ↗

(On Diff #62049)

This seems really error-prone and likely to cause a long tail of bugs where we need to add stuff to the list (at the very least there should be a fatal error if we cannot infer). Think about it this way: ideally we will get the EMachine from the LTO object file later, but we need it earlier.
Can we somehow call into LLVM to get this information from MC? Maybe Rafael would know.

[ruiu]

Can you please review http://reviews.llvm.org/D21784? This change should make it easier to do the same thing for bitcode files.

[grimar]

I faced with close problem yesterday: a need of parcing the triples.
I ended up with the patch below. I do not think it is acceptable as final solution as it does
not check every bitcode file, but checks the LLD-INTERNAL-combined-lto-object result.
Though it works. Here just for completeness :)

D21786

[grimar]

ELF/InputFiles.cpp

699 ↗

(On Diff #62049)

Seems you're not using the T.
What about

if (!TargetRegistry::lookupTarget(TripleStr, Msg))
  fatal("target not found: " + Msg);

703 ↗

(On Diff #62049)

You're using IsLE and DL only once, I think this can be replaced with

if (M.getDataLayout().isLittleEndian())

[mehdi_amini]

ELF/InputFiles.cpp

694 ↗

(On Diff #62049)

There is an API to get the Triple from a memory buffer without parsing the IR (I don't know if you need a module for something else here).

[davide]

Rewrite after Rui's refactoring patch went in

[davide]

Another possibility is to pass the triple to the function so that the computation of the triple from MemoryBuffer is shared in the constructor. I can change if you like that better.

ELF/InputFiles.cpp

723 ↗

(On Diff #62174)

Used, thanks Mehdi!

736 ↗

(On Diff #62174)

Done, thanks!

737 ↗

(On Diff #62174)

Yes, unfortunately :( At least now we error out so we should able to catch those. I talked briefly with Rafael on IRC and he said we probably need a switch(), hopefully we can move away from it at some point.

[ruiu]

ELF/InputFiles.cpp

556 ↗

(On Diff #62174)

M -> MB by convention.

556 ↗

(On Diff #62174)

Can this be a non-member static function?

561–562 ↗

(On Diff #62174)

Remove () surrounding Is64Bits.

565 ↗

(On Diff #62174)

Ditto (M -> MB and non-member static function.)

568 ↗

(On Diff #62174)

Can this be Triple(TripleStr).getArch())?

569–570 ↗

(On Diff #62174)

Move this at end of this switch and inline fatal()

default:
  fatal("...");

592 ↗

(On Diff #62174)

M -> MB

[ruiu]

ELF/InputFiles.cpp

589 ↗

(On Diff #62174)

I'd include the triple to the error message.

fatal("unsupported architecture: " + TripleStr);

[davide]

Rui's style comments addressed.

[ruiu]

LGTM with one more nit.

ELF/InputFiles.cpp

568 ↗

(On Diff #62178)

Remove llvm:: from all cases.

[dsanders]

lld/trunk/ELF/InputFiles.cpp

559–562

Sorry for the late comment but Triple::isArch64Bit() doesn't correspond to ELF64 or ELF32 on MIPS. In particular, the triple used for the N32 ABI will return true for isArch64Bit() but this ABI uses ELF32 objects. If lld doesn't support the N32 ABI yet then the current code is ok for the remaining two ABI's (O32 and N64).

[davide]

lld/trunk/ELF/InputFiles.cpp

559–562

+ Simon as he wrote the MIPS support.

[llvm-commits]

From: Rui Ueyama [[e-mail]]
Sent: 06 July 2016 18:24
To: [e-mail]
Cc: Davide Italiano; Rafael Ávila de Espíndola; Daniel Sanders; George Rimar; Sean Silva; Peter Collingbourne; llvm-commits; Tim Amini Golling
Subject: Re: [PATCH] D21779: [LTO] Infer EKind/EMachine from Bitcode files

On Tue, Jul 5, 2016 at 6:33 AM, Daniel Sanders <[e-mail]<mailto:[e-mail]>> wrote:
dsanders added a subscriber: dsanders.

Comment at: lld/trunk/ELF/InputFiles.cpp:559-562
@@ +558,6 @@
+ Triple TheTriple(TripleStr);
+ bool Is64Bits = TheTriple.isArch64Bit();
+ if (TheTriple.isLittleEndian())
+ return Is64Bits ? ELF64LEKind : ELF32LEKind;
+ return Is64Bits ? ELF64BEKind : ELF32BEKind;

+}

Sorry for the late comment but Triple::isArch64Bit() doesn't correspond to ELF64 or ELF32 on MIPS. In particular, the triple used for the
N32 ABI will return true for isArch64Bit() but this ABI uses ELF32 objects. If lld doesn't support the N32 ABI yet then the current code is
ok for the remaining two ABI's (O32 and N64).

Ah, interesting. So N32 is a 32-bit ABI on 64-bit MIPS just like x32 ABI is on x86-64. Do you know how to determine whether it is ELF32 or ELF64 for such triple?

In some ways, yes, but N32 has full support for 64-bit arithmetic. It was created to be similar to N64 but avoid the portability problems and bloat associated with 64-bit pointers.

It's not possible to distinguish N32 from N64 using the triple alone but I have patches under review to make this possible (D21465, D21467, D21069 for LLVM along with D21070, D21072 for clang). They're currently being discussed on llvm-dev in the 'Representing MIPS ABI information in the triple as ARM/X86 do for EABI/EABIHF/X32' thread. Assuming these patches are accepted, you'll be able to distinguish them using Triple::getEnvironment().

---

llvm-commits mailing list
[e-mail]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

[dsanders]

Ah, interesting. So N32 is a 32-bit ABI on 64-bit MIPS just like x32 ABI is on x86-64. Do you know how to determine whether it is ELF32 or ELF64 for such triple?

In some ways, yes, but N32 has full support for 64-bit arithmetic. It was created to be similar to N64 but avoid the portability problems and bloat associated with 64-bit pointers.

I've just noticed I've misread the table I was looking at. You're right, N32 serves the same purpose for 64-bit MIPS as X32 does for x86-64.

[dsanders]

On Thu, Jul 7, 2016 at 6:45 AM, Daniel Sanders <[e-mail]<mailto:[e-mail]>> wrote:
dsanders added a comment.

Ah, interesting. So N32 is a 32-bit ABI on 64-bit MIPS just like x32 ABI is on x86-64. Do you know how to determine whether it is ELF32 or ELF64 for such triple?

In some ways, yes, but N32 has full support for 64-bit arithmetic. It was created to be similar to N64 but avoid the portability problems and bloat associated with 64-bit pointers.

I've just noticed I've misread the table I was looking at. You're right, N32 serves the same purpose for 64-bit MIPS as X32 does for x86-64.

Yup, x32 is not a popular ABI but it exists and different from i386 ABI. :) It made me wonder if we have the same issue for x32. Will your patch address the issue for x32 as well? (I'm just wondering because we don't support x32 in LLD yet.)

It doesn't look like X32 has the same problem. In fact, X86 seems to have solved the problem in almost the exact same way as my current patch series :-). The only difference being that it's adjusting the triple in the clang driver instead of cc1 and cc1as. The driver is probably the right place for it but I had trouble with the adjusted triple ending up in library paths when I tried it that way. I'll mention this in the other thread and hopefully that will unblock my patch series. I'll take another look at doing it in the driver while I'm waiting for people to reply.

You should be able to use 'Triple::getEnvironment() == Triple::GNUX32' to identify X32 since the current implementation computeTargetTriple() in the driver switches between Triple::GNUX32, Triple::GNU, and Triple::CODE16 to match the -m16/-m32/-mx32/-m64 options.

lld/trunk/ELF/InputFiles.cpp

 //===- InputFiles.cpp -----------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "InputFiles.h"
 #include "Driver.h"
 #include "Error.h"
 #include "InputSection.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
for (const Elf_Sym &Sym : Syms) {
     }
 
     const Elf_Verdef *V =
         VersymIndex == VER_NDX_GLOBAL ? nullptr : Verdefs[VersymIndex];
     elf::Symtab<ELFT>::X->addShared(this, Name, Sym, V);
   }
 }
 
-BitcodeFile::BitcodeFile(MemoryBufferRef M) : InputFile(BitcodeKind, M) {}
+static ELFKind getELFKind(MemoryBufferRef MB) {
+  std::string TripleStr = getBitcodeTargetTriple(MB, Driver->Context);
+  Triple TheTriple(TripleStr);
+  bool Is64Bits = TheTriple.isArch64Bit();
+  if (TheTriple.isLittleEndian())
+    return Is64Bits ? ELF64LEKind : ELF32LEKind;
+  return Is64Bits ? ELF64BEKind : ELF32BEKind;
+}
+
+static uint8_t getMachineKind(MemoryBufferRef MB) {
+  std::string TripleStr = getBitcodeTargetTriple(MB, Driver->Context);
+  switch (Triple(TripleStr).getArch()) {
+    case Triple::aarch64:
+      return EM_AARCH64;
+    case Triple::arm:
+      return EM_ARM;
+    case Triple::mips:
+    case Triple::mipsel:
+    case Triple::mips64:
+    case Triple::mips64el:
+      return EM_MIPS;
+    case Triple::ppc:
+      return EM_PPC;
+    case Triple::ppc64:
+      return EM_PPC64;
+    case Triple::x86:
+      return EM_386;
+    case Triple::x86_64:
+      return EM_X86_64;
+    default:
+      fatal("unsupported architecture: " + TripleStr);
+  }
+}
+
+BitcodeFile::BitcodeFile(MemoryBufferRef MB) :
+    InputFile(BitcodeKind, MB) {
+  EKind = getELFKind(MB);
+  EMachine = getMachineKind(MB);
+}
 
 static uint8_t getGvVisibility(const GlobalValue *GV) {
   switch (GV->getVisibility()) {
   case GlobalValue::DefaultVisibility:
     return STV_DEFAULT;
   case GlobalValue::HiddenVisibility:
     return STV_HIDDEN;
   case GlobalValue::ProtectedVisibility:

lld/trunk/test/ELF/lto/pic.ll

 ; REQUIRES: x86
 
 ; RUN: llvm-as %s -o %t.o
-; RUN: ld.lld %t.o -m elf_x86_64 -o %t.so -shared
+; RUN: ld.lld %t.o -o %t.so -shared
 ; RUN: llvm-readobj -r %t.so | FileCheck %s
 
 ; CHECK:      Relocations [
 ; CHECK-NEXT:   Section ({{.*}}) .rela.plt {
 ; CHECK-NEXT:     R_X86_64_JUMP_SLOT bar 0x0
 ; CHECK-NEXT:   }
 ; CHECK-NEXT: ]
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 declare void @bar()
 define void @foo() {
   call void @bar()
   ret void
 }