[ThinLTO] Do not assert when adding a module with a different but compatible target triple

[ahatanak]

Summary

Currently, an assertion fails in ThinLTOCodeGenerator::addModule when the target triple of the module being added doesn't match that of the one stored in TMBuilder. This patch relaxes the constraint and makes changes to allow target triples that only differ in their version numbers on Apple platforms, similarly to what r228999 did.

Note that this patch only checks whether the target triples of the modules are compatible, but doesn't do anything to change the target triple in the IR when different but compatible triples are seen. Should we overwrite the triples in the IR so that they are identical to the one in TargetMachine? If the answer is yes, where or when should that happen?

rdar://problem/30133904

[mehdi_amini]

LGTM. See one inline comment.

include/llvm/ADT/Triple.h

730 ↗

(On Diff #99328)

match isn't very clear to me, I wouldn't know the difference between "match" and "equal", do you have a better name?
I'd suggest isCompatibleWith or something like this.

[ahatanak]

Renamed function to isCompatibleWith. Also, added FileChecks to the test case.

llvm/trunk/include/llvm/ADT/Triple.h

public:
 
   bool operator==(const Triple &Other) const {
     return Arch == Other.Arch && SubArch == Other.SubArch &&
            Vendor == Other.Vendor && OS == Other.OS &&
            Environment == Other.Environment &&
            ObjectFormat == Other.ObjectFormat;
   }
 
+  bool operator!=(const Triple &Other) const {
+    return !(*this == Other);
+  }
+
   /// @}
   /// @name Normalization
   /// @{
 
   /// normalize - Turn an arbitrary machine specification into the canonical
   /// triple form (or something sensible that the Triple class understands if
   /// nothing better can reasonably be done).  In particular, it handles the
   /// common case in which otherwise valid components are in the wrong order.
public:
   /// string then the triple's arch name is used.
   StringRef getARMCPUForArch(StringRef Arch = StringRef()) const;
 
   /// Tests whether the target triple is little endian.
   ///
   /// \returns true if the triple is little endian, false otherwise.
   bool isLittleEndian() const;
 
+  /// Test whether target triples are compatible.
+  bool isCompatibleWith(const Triple &Other) const;
+
+  /// Merge target triples.
+  std::string merge(const Triple &Other) const;
+
   /// @}
   /// @name Static helpers for IDs.
   /// @{
 
   /// getArchTypeName - Get the canonical name for the \p Kind architecture.
   static StringRef getArchTypeName(ArchType Kind);
 
   /// getArchTypePrefix - Get the "prefix" canonical name for the \p Kind

llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp

static void initTMBuilder(TargetMachineBuilder &TMBuilder,
   }
   TMBuilder.TheTriple = std::move(TheTriple);
 }
 
 } // end anonymous namespace
 
 void ThinLTOCodeGenerator::addModule(StringRef Identifier, StringRef Data) {
   ThinLTOBuffer Buffer(Data, Identifier);
-  if (Modules.empty()) {
-    // First module added, so initialize the triple and some options
-    LLVMContext Context;
-    StringRef TripleStr;
-    ErrorOr<std::string> TripleOrErr = expectedToErrorOrAndEmitErrors(
-        Context, getBitcodeTargetTriple(Buffer.getMemBuffer()));
-    if (TripleOrErr)
-      TripleStr = *TripleOrErr;
-    Triple TheTriple(TripleStr);
+
+
+
+  if (Modules.empty())
     initTMBuilder(TMBuilder, Triple(TheTriple));
+  else if (TMBuilder.TheTriple != TheTriple) {
+    if (!TMBuilder.TheTriple.isCompatibleWith(TheTriple))
+      report_fatal_error("ThinLTO modules with incompatible triples not "
+                         "supported");
+    initTMBuilder(TMBuilder, Triple(TMBuilder.TheTriple.merge(TheTriple)));
   }
-#ifndef NDEBUG
-  else {
-    LLVMContext Context;
-    StringRef TripleStr;
-    ErrorOr<std::string> TripleOrErr = expectedToErrorOrAndEmitErrors(
-        Context, getBitcodeTargetTriple(Buffer.getMemBuffer()));
-    if (TripleOrErr)
-      TripleStr = *TripleOrErr;
-    assert(TMBuilder.TheTriple.str() == TripleStr &&
-           "ThinLTO modules with different triple not supported");
-  }
-#endif
+
   Modules.push_back(Buffer);
 }
 
 void ThinLTOCodeGenerator::preserveSymbol(StringRef Name) {
   PreservedSymbols.insert(Name);
 }
 
 void ThinLTOCodeGenerator::crossReferenceSymbol(StringRef Name) {

llvm/trunk/lib/Linker/IRMover.cpp

for (unsigned I = 0, E = Requirements.size(); I != E; ++I) {
     MDNode *Op = Flags[Flag].first;
     if (!Op || Op->getOperand(2) != ReqValue)
       return stringErr("linking module flags '" + Flag->getString() +
                        "': does not have the required value");
   }
   return Error::success();
 }
 
-// This function returns true if the triples match.
-static bool triplesMatch(const Triple &T0, const Triple &T1) {
-  // If vendor is apple, ignore the version number.
-  if (T0.getVendor() == Triple::Apple)
-    return T0.getArch() == T1.getArch() && T0.getSubArch() == T1.getSubArch() &&
-           T0.getVendor() == T1.getVendor() && T0.getOS() == T1.getOS();
-
-  return T0 == T1;
-}
-
-// This function returns the merged triple.
-static std::string mergeTriples(const Triple &SrcTriple,
-                                const Triple &DstTriple) {
-  // If vendor is apple, pick the triple with the larger version number.
-  if (SrcTriple.getVendor() == Triple::Apple)
-    if (DstTriple.isOSVersionLT(SrcTriple))
-      return SrcTriple.str();
-
-  return DstTriple.str();
-}
-
 Error IRLinker::run() {
   // Ensure metadata materialized before value mapping.
   if (SrcM->getMaterializer())
     if (Error Err = SrcM->getMaterializer()->materializeMetadata())
       return Err;
 
   // Inherit the target data from the source module if the destination module
   // doesn't have one already.
Error IRLinker::run() {
   }
 
   // Copy the target triple from the source to dest if the dest's is empty.
   if (DstM.getTargetTriple().empty() && !SrcM->getTargetTriple().empty())
     DstM.setTargetTriple(SrcM->getTargetTriple());
 
   Triple SrcTriple(SrcM->getTargetTriple()), DstTriple(DstM.getTargetTriple());
 
-  if (!SrcM->getTargetTriple().empty() && !triplesMatch(SrcTriple, DstTriple))
+  if (!SrcM->getTargetTriple().empty()&&
+      !SrcTriple.isCompatibleWith(DstTriple))
     emitWarning("Linking two modules of different target triples: " +
                 SrcM->getModuleIdentifier() + "' is '" +
                 SrcM->getTargetTriple() + "' whereas '" +
                 DstM.getModuleIdentifier() + "' is '" + DstM.getTargetTriple() +
                 "'\n");
 
-  DstM.setTargetTriple(mergeTriples(SrcTriple, DstTriple));
+  DstM.setTargetTriple(SrcTriple.merge(DstTriple));
 
   // Append the module inline asm string.
   if (!IsPerformingImport && !SrcM->getModuleInlineAsm().empty()) {
     if (DstM.getModuleInlineAsm().empty())
       DstM.setModuleInlineAsm(SrcM->getModuleInlineAsm());
     else
       DstM.setModuleInlineAsm(DstM.getModuleInlineAsm() + "\n" +
                               SrcM->getModuleInlineAsm());

llvm/trunk/lib/Support/Triple.cpp

bool Triple::isLittleEndian() const {
   case Triple::renderscript32:
   case Triple::renderscript64:
     return true;
   default:
     return false;
   }
 }
 
+bool Triple::isCompatibleWith(const Triple &Other) const {
+  // If vendor is apple, ignore the version number.
+  if (getVendor() == Triple::Apple)
+    return getArch() == Other.getArch() && getSubArch() == Other.getSubArch() &&
+           getVendor() == Other.getVendor() && getOS() == Other.getOS();
+
+  return *this == Other;
+}
+
+std::string Triple::merge(const Triple &Other) const {
+  // If vendor is apple, pick the triple with the larger version number.
+  if (getVendor() == Triple::Apple)
+    if (Other.isOSVersionLT(*this))
+      return str();
+
+  return Other.str();
+}
+
 StringRef Triple::getARMCPUForArch(StringRef MArch) const {
   if (MArch.empty())
     MArch = getArchName();
   MArch = ARM::getCanonicalArchName(MArch);
 
   // Some defaults are forced.
   switch (getOS()) {
   case llvm::Triple::FreeBSD:

llvm/trunk/test/ThinLTO/X86/Inputs/merge-triple.ll

+target triple = "x86_64-apple-macosx10.11.0"

llvm/trunk/test/ThinLTO/X86/merge-triple.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/merge-triple.ll -o %t2.bc
+; RUN: llvm-lto -thinlto-action=optimize %t1.bc %t2.bc
+; RUN: llvm-dis < %t1.bc.thinlto.imported.bc | FileCheck %s --check-prefix=CHECK1
+; RUN: llvm-dis < %t2.bc.thinlto.imported.bc | FileCheck %s --check-prefix=CHECK2
+
+target triple = "x86_64-apple-macosx10.12.0"
+
+; CHECK1: target triple = "x86_64-apple-macosx10.12.0"
+; CHECK2: target triple = "x86_64-apple-macosx10.11.0"