[lld-macho] Run ObjCContractPass during LTO

[int3]

Summary

Run the ObjCARCContractPass during LTO. The legacy LTO backend (under
LTO/ThinLTOCodeGenerator.cpp) already does this; this diff just adds that
behavior to the new LTO backend. Without that pass, the objc.clang.arc.use
intrinsic will get passed to the instruction selector, which doesn't know how to
handle it.

In order to test both the new and old pass managers, I've also added support for
the --[no-]lto-legacy-pass-manager flags.

P.S. Not sure if the ordering of the pass within the pipeline matters...

[lanza]

This goes right before codegen in ThinLTOCodeGenerator. TM.addPassesToEmitFile is the entry point to the llvm backend. So the legacy implementation has this pass ran immediately after the front-end is finished and right before the backend starts. I imagine doing the same here makes the most sense.

[int3]

add passes at the end of the pipeline

[lanza]

215d59e7b0b14

llvm/lib/LTO/LTOBackend.cpp

426

Sorry, to be clearer I think around here is the analogous point for the LTO.cpp pipeline. It directly precedes addPassesToEmitFile in the legacy impl.

[lanza]

llvm/lib/LTO/LTOBackend.cpp

426

codegen and optimizations aren't directly coupled. You can codegen without optimizing and vice versa and run optimizations multiple times in the legacy impl. So this pass should stay coupled to codegen.

[fhahn]

llvm/lib/LTO/LTOBackend.cpp

282

The reason this is not added to all backend pipelines is (presumably) that we do not want to run this unnecessarily on platforms that do not support ObjC. I think we should add a new flag to lto::Config, so users can request it. But as I mentioned below, I think it would be better to do it at the beginning of the codegen pass pipeline.

426

Yes, you don't necessarily have to run the optimize stage, but we still need to ensure the pass is run before codegen. So I think here would be a suitable place to just add the pass to CodeGenPasses (LTOCodeGenerator.cpp runs it just before calling splitCodeGen https://github.com/llvm/llvm-project/blob/master/llvm/lib/LTO/LTOCodeGenerator.cpp#L620)

[int3]

llvm/lib/LTO/LTOBackend.cpp

282

Isn't it technically possible for every platform to support ObjC, e.g. with GNUstep?

[int3]

llvm/lib/LTO/LTOBackend.cpp

282

(But I suppose it's a good idea not to make folks pay for edge cases that are in practice never used)

[fhahn]

llvm/lib/LTO/LTOBackend.cpp

282

Isn't it technically possible for every platform to support ObjC, e.g. with GNUstep?

I think all the matters in this case are frontends that emit LLVM IR I think. Not sure if Clang + ObjC is actually usable on non-Darwin platforms in practice.

In any case, I think it would be worth making it opt-in to start with. And if you are looking for enabling it unconditionally, it would probably be good as a separate patch and with a concrete motivation behind it.

[int3]

run pass right before codegen via a config hook

[int3]

update

[int3]

Since we're only running the ObjCContractPass via the one pass manager in codegen, there's actually no real need to add the --[no-]lto-legacy-pass-manager flags in this diff. But it probably doesn't hurt to leave it in, along with the test, just to make sure that any future migrations to the non-legacy PM don't break behavior

llvm/include/llvm/LTO/Config.h

47

I initially tried making this a vector of Pass *, but it appears that creating the ObjCContractPass too early results in an error later:

Assertion failed: (!Resolver && "Resolver is already set"), function setResolver, file /Users/jezng/src/llvm-project/llvm/lib/IR/Pass.cpp

Using this hook to delay initialization seems to work, though I'm not sure if it's the right solution.

[fhahn]

llvm/include/llvm/LTO/Config.h

47

Can you a doc-comment for PreCodeGenPassesHook?

llvm/lib/LTO/LTOBackend.cpp

435

Can we just keep the name of the pass manager as CodeGenPasses?

[int3]

llvm/include/llvm/LTO/Config.h

47

For sure, was planning to do that. Just wanted to make sure that using a hook to delay initialization was the right approach here...

llvm/lib/LTO/LTOBackend.cpp

435

I'd thought that since we are now including pre-codegen passes in it, the old name was a bit of a misnomer... but I don't feel strongly about it

[int3]

address comments

[fhahn]

LGTM, thanks!

llvm/include/llvm/LTO/Config.h

47

nit: /// for doc-comment

lld/MachO/Config.h

struct Configuration {
   Symbol *entry;
   bool hasReexports = false;
   bool allLoad = false;
   bool forceLoadObjC = false;
   bool staticLink = false;
   bool implicitDylibs = false;
   bool isPic = false;
   bool headerPadMaxInstallNames = false;
+  bool ltoNewPassManager = LLVM_ENABLE_NEW_PASS_MANAGER;
   bool printEachFile = false;
   bool printWhyLoad = false;
   bool searchDylibsFirst = false;
   bool saveTemps = false;
   uint32_t headerPad;
   uint32_t dylibCompatibilityVersion = 0;
   uint32_t dylibCurrentVersion = 0;
   llvm::StringRef installName;

lld/MachO/Driver.cpp

config->installName =
       args.getLastArgValue(OPT_install_name, config->outputFile);
   config->headerPad = args::getHex(args, OPT_headerpad, /*Default=*/32);
   config->headerPadMaxInstallNames =
       args.hasArg(OPT_headerpad_max_install_names);
   config->printEachFile = args.hasArg(OPT_t);
   config->printWhyLoad = args.hasArg(OPT_why_load);
   config->outputType = getOutputType(args);
   config->ltoObjPath = args.getLastArgValue(OPT_object_path_lto);
+  config->ltoNewPassManager =
+      args.hasFlag(OPT_no_lto_legacy_pass_manager, OPT_lto_legacy_pass_manager,
+                   LLVM_ENABLE_NEW_PASS_MANAGER);
   config->runtimePaths = args::getStrings(args, OPT_rpath);
   config->allLoad = args.hasArg(OPT_all_load);
   config->forceLoadObjC = args.hasArg(OPT_ObjC);
   config->demangle = args.hasArg(OPT_demangle);
   config->implicitDylibs = !args.hasArg(OPT_no_implicit_dylibs);
 
   if (const opt::Arg *arg = args.getLastArg(OPT_static, OPT_dynamic))
     config->staticLink = (arg->getOption().getID() == OPT_static);

lld/MachO/LTO.cpp

 
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Strings.h"
 #include "lld/Common/TargetOptionsCommandFlags.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/ObjCARC.h"
 
 using namespace lld;
 using namespace lld::macho;
 using namespace llvm;
 using namespace llvm::sys;
 
 static lto::Config createConfig() {
   lto::Config c;
   c.Options = initTargetOptionsFromCodeGenFlags();
   c.CodeModel = getCodeModelFromCMModel();
   c.CPU = getCPUStr();
   c.MAttrs = getMAttrs();
+  c.UseNewPM = config->ltoNewPassManager;
+  c.PreCodeGenPassesHook = [](legacy::PassManager &pm) {
+    pm.add(createObjCARCContractPass());
+  };
   return c;
 }
 
 BitcodeCompiler::BitcodeCompiler() {
   auto backend =
       lto::createInProcessThinBackend(heavyweight_hardware_concurrency());
   ltoObj = std::make_unique<lto::LTO>(createConfig(), backend);
 }

lld/MachO/Options.td

def reproduce: Separate<["--"], "reproduce">,
     Group<grp_lld>;
 def reproduce_eq: Joined<["--"], "reproduce=">,
     Alias<!cast<Separate>(reproduce)>,
     HelpText<"Write tar file containing inputs and command to reproduce link">,
     Group<grp_lld>;
 def version: Flag<["--"], "version">,
     HelpText<"Display the version number and exit">,
     Group<grp_lld>;
+def lto_legacy_pass_manager: Flag<["--"], "lto-legacy-pass-manager">,
+    HelpText<"Use the legacy pass manager in LLVM">,
+    Group<grp_lld>;
+def no_lto_legacy_pass_manager : Flag<["--"], "no-lto-legacy-pass-manager">,
+    HelpText<"Use the new pass manager in LLVM">,
+    Group<grp_lld>;
 
 
 // This is a complete Options.td compiled from Apple's ld(1) manpage
 // dated 2018-03-07 and cross checked with ld64 source code in repo
 // https://github.com/apple-opensource/ld64 at git tag "512.4" dated
 // 2018-03-18.
 
 // Flags<[HelpHidden]> marks options that are not yet ported to lld,

lld/test/MachO/objc-arc-contract.ll

+; REQUIRES: x86
+
+;; Verify that we run the ObjCARCContractPass during LTO. Without that, the
+;; objc.clang.arc.use intrinsic will get passed to the instruction selector,
+;; which doesn't know how to handle it.
+
+; RUN: llvm-as %s -o %t.o
+; RUN: %lld -dylib -lSystem %t.o -o %t --lto-legacy-pass-manager
+; RUN: llvm-objdump -d %t | FileCheck %s
+; RUN: %lld -dylib -lSystem %t.o -o %t --no-lto-legacy-pass-manager
+; RUN: llvm-objdump -d %t | FileCheck %s
+
+; RUN: opt -module-summary %s -o %t.o
+; RUN: %lld -dylib -lSystem %t.o -o %t --lto-legacy-pass-manager
+; RUN: llvm-objdump -d %t | FileCheck %s
+; RUN: %lld -dylib -lSystem %t.o -o %t --no-lto-legacy-pass-manager
+; RUN: llvm-objdump -d %t | FileCheck %s
+
+; CHECK:      <_foo>:
+; CHECK-NEXT: retq
+
+target triple = "x86_64-apple-darwin"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+define void @foo(i8* %a, i8* %b) {
+  call void (...) @llvm.objc.clang.arc.use(i8* %a, i8* %b) nounwind
+  ret void
+}
+
+declare void @llvm.objc.clang.arc.use(...) nounwind

llvm/include/llvm/LTO/Config.h

 #ifndef LLVM_LTO_CONFIG_H
 #define LLVM_LTO_CONFIG_H
 
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/LegacyPassManager.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Target/TargetOptions.h"
 
 #include <functional>
 
 namespace llvm {
 
 class Error;
 class Module;
 class ModuleSummaryIndex;
 class raw_pwrite_stream;
 
 namespace lto {
 
 /// LTO configuration. A linker can configure LTO by setting fields in this data
 /// structure and passing it to the lto::LTO constructor.
 struct Config {
   // Note: when adding fields here, consider whether they need to be added to
   // computeCacheKey in LTO.cpp.
   std::string CPU;
   TargetOptions Options;
   std::vector<std::string> MAttrs;
   std::vector<std::string> PassPlugins;
+  /// For adding passes that run right before codegen.
+  std::function<void(legacy::PassManager &)> PreCodeGenPassesHook;
   Optional<Reloc::Model> RelocModel = Reloc::PIC_;
   Optional<CodeModel::Model> CodeModel = None;
   CodeGenOpt::Level CGOptLevel = CodeGenOpt::Default;
   CodeGenFileType CGFileType = CGFT_ObjectFile;
   unsigned OptLevel = 2;
   bool DisableVerify = false;
 
   /// Use the new pass manager

llvm/lib/LTO/LTOBackend.cpp

static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
   if (IsThinLTO)
     MPM.addPass(PB.buildThinLTODefaultPipeline(OL, ImportSummary));
   else
     MPM.addPass(PB.buildLTODefaultPipeline(OL, ExportSummary));
 
   if (!Conf.DisableVerify)
     MPM.addPass(VerifierPass());
 
   MPM.run(Mod, MAM);
 }
 
 static void runNewPMCustomPasses(const Config &Conf, Module &Mod,
                                  TargetMachine *TM, std::string PipelineDesc,
                                  std::string AAPipelineDesc,
                                  bool DisableVerify) {
   PassBuilder PB(Conf.DebugPassManager, TM);
   AAManager AA;
if (!Conf.DwoDir.empty()) {
     TM->Options.MCOptions.SplitDwarfFile = Conf.SplitDwarfFile;
 
   if (!DwoFile.empty()) {
     std::error_code EC;
     DwoOut = std::make_unique<ToolOutputFile>(DwoFile, EC, sys::fs::OF_None);
     if (EC)
       report_fatal_error("Failed to open " + DwoFile + ": " + EC.message());
   }
 
   auto Stream = AddStream(Task);
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
       createImmutableModuleSummaryIndexWrapperPass(&CombinedIndex));
+  if (Conf.PreCodeGenPassesHook)
+    Conf.PreCodeGenPassesHook(CodeGenPasses);
   if (TM->addPassesToEmitFile(CodeGenPasses, *Stream->OS,
                               DwoOut ? &DwoOut->os() : nullptr,
                               Conf.CGFileType))
     report_fatal_error("Failed to setup codegen");
   CodeGenPasses.run(Mod);
 
   if (DwoOut)
     DwoOut->keep();
 }
 
 static void splitCodeGen(const Config &C, TargetMachine *TM,