[Driver] Infer the correct option to ld64 for -fembed-bitcode

[steven_wu]

Summary

-fembed-bitcode infers -bitcode_bundle to ld64 but it is not correctly
passed when using LTO. LTO is a special case of -fembed-bitcode which
it doesn't require embed the bitcode in a special section in the object
file but it requires linker to save that as part of the final executable.

rdar://problem/29274226

[mehdi_amini]

lib/Driver/Tools.cpp

8326 ↗

(On Diff #78051)

Why aren't these tests returning true when LTO is enabled?
I'm not sure why this is not the part that should be fixed instead.

[steven_wu]

lib/Driver/Tools.cpp

8326 ↗

(On Diff #78051)

These flags controls if there should be bitcode embedded in the object file. For the case of LTO, there is no object file thus there is no embedding.
If change the meaning of this flag, it will complicate the logic around line 6355 and 4169. The other option is to add another flag but it seems little too much for passing one flag to ld.

[mehdi_amini]

lib/Driver/Tools.cpp

8326 ↗

(On Diff #78051)

From the driver point of view, having a method embedBitcodeEnabled() returning false when ... it is actually enabled does not make sense to me.

At minima it should be renamed.

[steven_wu]

Rename the functions to clarify the intention.
embedBitcodeEnabled means if any flag in -fembed-bitcode family is used so the
corresponding linker flag should be inferred. embedBitcodeInObject means if
the bitcode should be embedded in the object file which returns false for LTO.

[mehdi_amini]

LGTM!

I feel the code is cleaner like that at uses of these API :)

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

public:
   }
   void setInstalledDir(StringRef Value) {
     InstalledDir = Value;
   }
 
   bool isSaveTempsEnabled() const { return SaveTemps != SaveTempsNone; }
   bool isSaveTempsObj() const { return SaveTemps == SaveTempsObj; }
 
-  bool embedBitcodeEnabled() const { return BitcodeEmbed == EmbedBitcode; }
-  bool embedBitcodeMarkerOnly() const { return BitcodeEmbed == EmbedMarker; }
+  bool embedBitcodeEnabled() const { return BitcodeEmbed != EmbedNone; }
+  bool embedBitcodeInObject() const {
+    // LTO has no object file output so ignore embed bitcode option in LTO.
+    return (BitcodeEmbed == EmbedBitcode) && !isUsingLTO();
+  }
+  bool embedBitcodeMarkerOnly() const {
+    return (BitcodeEmbed == EmbedMarker) && !isUsingLTO();
+  }
 
   /// Compute the desired OpenMP runtime from the flags provided.
   OpenMPRuntimeKind getOpenMPRuntime(const llvm::opt::ArgList &Args) const;
 
   /// @}
   /// @name Primary Functionality
   /// @{
 

cfe/trunk/lib/Driver/Driver.cpp

if (const Arg *A = Args.getLastArg(options::OPT_save_temps_EQ)) {
     SaveTemps = llvm::StringSwitch<SaveTempsMode>(A->getValue())
                     .Case("cwd", SaveTempsCwd)
                     .Case("obj", SaveTempsObj)
                     .Default(SaveTempsCwd);
   }
 
   setLTOMode(Args);
 
-  // Ignore -fembed-bitcode options with LTO
-  // since the output will be bitcode anyway.
-  if (getLTOMode() == LTOK_None) {
-    if (Arg *A = Args.getLastArg(options::OPT_fembed_bitcode_EQ)) {
-      StringRef Name = A->getValue();
-      unsigned Model = llvm::StringSwitch<unsigned>(Name)
-          .Case("off", EmbedNone)
-          .Case("all", EmbedBitcode)
-          .Case("bitcode", EmbedBitcode)
-          .Case("marker", EmbedMarker)
-          .Default(~0U);
-      if (Model == ~0U) {
-        Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args)
-                                                  << Name;
-      } else
-        BitcodeEmbed = static_cast<BitcodeEmbedMode>(Model);
-    }
-  } else {
-    // claim the bitcode option under LTO so no warning is issued.
-    Args.ClaimAllArgs(options::OPT_fembed_bitcode_EQ);
-  }
+  // Process -fembed-bitcode= flags.
 
   std::unique_ptr<llvm::opt::InputArgList> UArgs =
       llvm::make_unique<InputArgList>(std::move(Args));
 
   // Perform the default argument translations.
   DerivedArgList *TranslatedArgs = TranslateInputArgs(*UArgs);
 
   // Owned by the host.
InputInfo Driver::BuildJobsForActionNoCache(
   }
 
 
   const ActionList *Inputs = &A->getInputs();
 
   const JobAction *JA = cast<JobAction>(A);
   ActionList CollapsedOffloadActions;
 
-  ToolSelector TS(JA, *TC, C, isSaveTempsEnabled(), embedBitcodeEnabled());
+  ToolSelector TS(JA, *TC, C, isSaveTempsEnabled(), embedBitcodeInObject());
   const Tool *T = TS.getTool(Inputs, CollapsedOffloadActions);
 
   if (!T)
     return InputInfo();
 
   // If we've collapsed action list that contained OffloadAction we
   // need to build jobs for host/device-side inputs it may have held.
   for (const auto *OA : CollapsedOffloadActions)

cfe/trunk/lib/Driver/Tools.cpp

void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   if (const Arg *A = Args.getLastArg(options::OPT_fthinlto_index_EQ)) {
     if (!types::isLLVMIR(Input.getType()))
       D.Diag(diag::err_drv_argument_only_allowed_with) << A->getAsString(Args)
                                                        << "-x ir";
     Args.AddLastArg(CmdArgs, options::OPT_fthinlto_index_EQ);
   }
 
   // Embed-bitcode option.
-  if (C.getDriver().embedBitcodeEnabled() &&
+  if (C.getDriver().embedBitcodeInObject() &&
       (isa<BackendJobAction>(JA) || isa<AssembleJobAction>(JA))) {
     // Add flags implied by -fembed-bitcode.
     Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
     // Disable all llvm IR level optimizations.
     CmdArgs.push_back("-disable-llvm-optzns");
   }
   if (C.getDriver().embedBitcodeMarkerOnly())
     CmdArgs.push_back("-fembed-bitcode=marker");
#endif
   // CompileJobAction, use -disable-llvm-passes to get pristine IR generated
   // by the frontend.
   // When -fembed-bitcode is enabled, optimized bitcode is emitted because it
   // has slightly different breakdown between stages.
   // FIXME: -fembed-bitcode -save-temps will save optimized bitcode instead of
   // pristine IR generated by the frontend. Ideally, a new compile action should
   // be added so both IR can be captured.
   if (C.getDriver().isSaveTempsEnabled() &&
-      !C.getDriver().embedBitcodeEnabled() && isa<CompileJobAction>(JA))
+      !C.getDriver().embedBitcodeInObject() && isa<CompileJobAction>(JA))
     CmdArgs.push_back("-disable-llvm-passes");
 
   if (Output.getType() == types::TY_Dependencies) {
     // Handled with other dependency code.
   } else if (Output.isFilename()) {
     CmdArgs.push_back("-o");
     CmdArgs.push_back(Output.getFilename());
   } else {
if (const Arg *A =
           Args.getLastArg(options::OPT_fpie, options::OPT_fPIE,
                           options::OPT_fno_pie, options::OPT_fno_PIE)) {
     if (A->getOption().matches(options::OPT_fpie) ||
         A->getOption().matches(options::OPT_fPIE))
       CmdArgs.push_back("-pie");
     else
       CmdArgs.push_back("-no_pie");
   }
+
   // for embed-bitcode, use -bitcode_bundle in linker command
-  if (C.getDriver().embedBitcodeEnabled() ||
-      C.getDriver().embedBitcodeMarkerOnly()) {
+  if (C.getDriver().embedBitcodeEnabled()) {
     // Check if the toolchain supports bitcode build flow.
     if (MachOTC.SupportsEmbeddedBitcode())
       CmdArgs.push_back("-bitcode_bundle");
     else
       D.Diag(diag::err_drv_bitcode_unsupported_on_toolchain);
   }
 
   Args.AddLastArg(CmdArgs, options::OPT_prebind);

cfe/trunk/test/Driver/embed-bitcode.c

 // CHECK-LTO-NOT: -fembed-bitcode=all
 
 // RUN: %clang -c %s -fembed-bitcode-marker -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-MARKER
 // CHECK-MARKER: -cc1
 // CHECK-MARKER: -emit-obj
 // CHECK-MARKER: -fembed-bitcode=marker
 // CHECK-MARKER-NOT: -cc1
 
+// RUN: %clang -target armv7-apple-darwin -miphoneos-version-min=6.0 %s -fembed-bitcode=all -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-LINKER
+// RUN: %clang -target armv7-apple-darwin -miphoneos-version-min=6.0 %s -fembed-bitcode=marker -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-LINKER
+// RUN: %clang -target armv7-apple-darwin -miphoneos-version-min=6.0 %s -flto=full -fembed-bitcode=bitcode -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-LINKER
+// RUN: %clang -target armv7-apple-darwin -miphoneos-version-min=6.0 %s -flto=thin -fembed-bitcode=bitcode -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-LINKER
+// RUN: %clang -target armv7-apple-darwin -miphoneos-version-min=6.0 %s -fembed-bitcode=off -fintegrated-as 2>&1 -### | FileCheck %s -check-prefix=CHECK-NO-LINKER
+// CHECK-LINKER: ld
+// CHECK-LINKER: -bitcode_bundle
+// CHECK-NO-LINKER-NOT: -bitcode_bundle
+
+// RUN: %clang -target armv7-apple-darwin -miphoneos-version-min=5.0 %s -fembed-bitcode -### 2>&1 | \
+// RUN:   FileCheck %s -check-prefix=CHECK-PLATFORM-UNSUPPORTED
+// CHECK-PLATFORM-UNSUPPORTED: -fembed-bitcode is not supported on versions of iOS prior to 6.0