Respect bound archs, even when they don't alter the toolchain.

[jlebar]

Summary

It's possible to BindArch without changing the toolchain at all. For
example, armv7 and armv7s have exactly the same triple.

Therefore the code in the Driver that checks that we're not creating a
job for the same Action twice needs to consider (Action, Toolchain,
BoundArch) tuples.

[echristo]

Ultimately they probably shouldn't map to the same toolchain and we can use
that as a key. Looks OK for now though.

[tra]

Looks OK to me.
Perhaps BoundArch w/o toolchain is sufficient for the key as toolchain would be derived from it.

[jlebar]

In D16250#328552, @tra wrote:

Looks OK to me.
Perhaps BoundArch w/o toolchain is sufficient for the key as toolchain would be derived from it.

Yeah, it might work, I wasn't convinced it was sound. The bound TC is derived from the original TC plus the arch -- that is, the arch isn't the only piece of info that goes into it. Dunno, once bitten twice shy.

Thank you for the reviews, and thanks again for your patience, Chris.

cfe/trunk/include/clang/Driver/Driver.h

public:
 
   /// ConstructAction - Construct the appropriate action to do for
   /// \p Phase on the \p Input, taking in to account arguments
   /// like -fsyntax-only or --analyze.
   Action *ConstructPhaseAction(Compilation &C, const ToolChain &TC,
                                const llvm::opt::ArgList &Args, phases::ID Phase,
                                Action *Input) const;
 
-  /// BuildJobsForAction - Construct the jobs to perform for the
-  /// action \p A and return an InputInfo for the result of running \p A.
-  /// Will only construct jobs for a given (Action, ToolChain) pair once.
+  /// BuildJobsForAction - Construct the jobs to perform for the action \p A and
+  /// return an InputInfo for the result of running \p A.  Will only construct
+  /// jobs for a given (Action, ToolChain, BoundArch) tuple once.
   InputInfo BuildJobsForAction(Compilation &C, const Action *A,
                                const ToolChain *TC, const char *BoundArch,
                                bool AtTopLevel, bool MultipleArchs,
                                const char *LinkingOutput,
                                std::map<std::pair<const Action *, std::string>,
                                         InputInfo> &CachedResults) const;
 
   /// Returns the default name for linked images (e.g., "a.out").

cfe/trunk/lib/Driver/Driver.cpp

static const Tool *selectToolForJob(Compilation &C, bool SaveTemps,
   return ToolForJob;
 }
 
 InputInfo Driver::BuildJobsForAction(
     Compilation &C, const Action *A, const ToolChain *TC, const char *BoundArch,
     bool AtTopLevel, bool MultipleArchs, const char *LinkingOutput,
     std::map<std::pair<const Action *, std::string>, InputInfo> &CachedResults)
     const {
-  std::pair<const Action *, std::string> ActionTC = {
-      A, TC->getTriple().normalize()};
+  // The bound arch is not necessarily represented in the toolchain's triple --
+  // for example, armv7 and armv7s both map to the same triple -- so we need
+  // both in our map.
+  std::string TriplePlusArch = TC->getTriple().normalize();
+  if (BoundArch) {
+    TriplePlusArch += "-";
+    TriplePlusArch += BoundArch;
+  }
+  std::pair<const Action *, std::string> ActionTC = {A, TriplePlusArch};
   auto CachedResult = CachedResults.find(ActionTC);
   if (CachedResult != CachedResults.end()) {
     return CachedResult->second;
   }
   InputInfo Result =
       BuildJobsForActionNoCache(C, A, TC, BoundArch, AtTopLevel, MultipleArchs,
                                 LinkingOutput, CachedResults);
   CachedResults[ActionTC] = Result;

cfe/trunk/test/Driver/darwin-multiarch-arm.c

+// Check that we compile correctly with multiple ARM -arch options.
+//
+// RUN: %clang -target arm7-apple-darwin10 -### \
+// RUN:   -arch armv7 -arch armv7s %s 2>&1 | FileCheck %s
+
+// CHECK: "-cc1" "-triple" "thumbv7-apple-ios5.0.0"
+// CHECK-SAME: "-o" "[[CC_OUT1:[^"]*]]"
+// CHECK:ld
+// CHECK-SAME: "-o" "[[LD_OUT1:[^"]*]]"
+// CHECK-SAME: "[[CC_OUT1]]"
+// CHECK:"-cc1" "-triple" "thumbv7s-apple-ios5.0.0"
+// CHECK-SAME: "-o" "[[CC_OUT2:[^"]*]]"
+// CHECK:ld
+// CHECK-SAME: "-o" "[[LD_OUT2:[^"]*]]"
+// CHECK-SAME: "[[CC_OUT2]]"
+// CHECK:lipo
+// CHECK-DAG: "[[LD_OUT1]]"
+// CHECK-DAG: "[[LD_OUT2]]"