[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

430

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

430

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

285

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.

430

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

285

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

[int3]

llvm/lib/LTO/LTOBackend.cpp

285

(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

285

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 ↗

(On Diff #317009)

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 ↗

(On Diff #317009)

Can you a doc-comment for PreCodeGenPassesHook?

llvm/lib/LTO/LTOBackend.cpp

437

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

[int3]

llvm/include/llvm/LTO/Config.h

47 ↗

(On Diff #317009)

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

437

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 ↗

(On Diff #317687)

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

 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;
   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/lib/LTO/LTOBackend.cpp

 #include "llvm/Support/Program.h"
 #include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
+#include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar/LoopPassManager.h"
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
 #include "llvm/Transforms/Utils/SplitModule.h"
 
 using namespace llvm;
 using namespace lto;
 
 #define DEBUG_TYPE "lto-backend"
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 (!DisableVerify)
     MPM.addPass(VerifierPass());
   MPM.run(Mod, MAM);
 }
 
 static void runOldPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
                            bool IsThinLTO, ModuleSummaryIndex *ExportSummary,
                            const ModuleSummaryIndex *ImportSummary) {
   legacy::PassManager passes;
+  passes.add(createObjCARCContractPass());
   passes.add(createTargetTransformInfoWrapperPass(TM->getTargetIRAnalysis()));
 
   PassManagerBuilder PMB;
   PMB.LibraryInfo = new TargetLibraryInfoImpl(Triple(TM->getTargetTriple()));
   PMB.Inliner = createFunctionInliningPass();
   PMB.ExportSummary = ExportSummary;
   PMB.ImportSummary = ImportSummary;
   // Unconditionally verify input since it is not verified before this
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 (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();
 }
 
 void splitCodeGen(const Config &C, TargetMachine *TM, AddStreamFn AddStream,

llvm/lib/Passes/PassBuilder.cpp

PassBuilder::buildThinLTOPreLinkDefaultPipeline(OptimizationLevel Level) {
 
   return MPM;
 }
 
 ModulePassManager PassBuilder::buildThinLTODefaultPipeline(
     OptimizationLevel Level, const ModuleSummaryIndex *ImportSummary) {
   ModulePassManager MPM(DebugLogging);
 
+  MPM.addPass(createModuleToFunctionPassAdaptor(ObjCARCContractPass()));
+
   // Convert @llvm.global.annotations to !annotation metadata.
   MPM.addPass(Annotation2MetadataPass());
 
   if (ImportSummary) {
     // These passes import type identifier resolutions for whole-program
     // devirtualization and CFI. They must run early because other passes may
     // disturb the specific instruction patterns that these passes look for,
     // creating dependencies on resolutions that may not appear in the summary.
return buildPerModuleDefaultPipeline(Level,
                                        /* LTOPreLink */ true);
 }
 
 ModulePassManager
 PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
                                      ModuleSummaryIndex *ExportSummary) {
   ModulePassManager MPM(DebugLogging);
 
+  MPM.addPass(createModuleToFunctionPassAdaptor(ObjCARCContractPass()));
+
   // Convert @llvm.global.annotations to !annotation metadata.
   MPM.addPass(Annotation2MetadataPass());
 
   if (Level == OptimizationLevel::O0) {
     // The WPD and LowerTypeTest passes need to run at -O0 to lower type
     // metadata and intrinsics.
     MPM.addPass(WholeProgramDevirtPass(ExportSummary, nullptr));
     MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr));