Migrate WholeProgramDevirt to new Optimization Remark API

[lenary]

Summary

This is an attempt to move WholeProgramDevirt to the new remark API.

https://bugs.llvm.org/show_bug.cgi?id=33793

[lenary]

This patch segfaults in the following three tests:

LLVM :: Transforms/WholeProgramDevirt/devirt-single-impl-check.ll
LLVM :: Transforms/WholeProgramDevirt/devirt-single-impl.ll
LLVM :: Transforms/WholeProgramDevirt/virtual-const-prop-check.ll

Unfortunately I'm right at the edge of my C++ abilities, and can't actually work out how to make this work. The segfault is in computeHotness which uses the BFI code. I think it could be because a unique_ptr is being dropped, maybe? Any help would be greatly appreciated.

[anemet]

lib/Transforms/IPO/WholeProgramDevirt.cpp

532–535

OwnedORE is freed here after the function returns and you're returning a pointer off of it.

I am not sure if we have any better options than just passing nullptr for OREGetter from the old PM path and build ORE at the use site if OREGetter is not set.

[lenary]

No more Segfaults!

[anemet]

LGTM with the nits below. We're done migrating!!!! Thanks for your work on this!

lib/Transforms/IPO/WholeProgramDevirt.cpp

1439

Remove this line.

1441–1448

A comment would be good, explaining the strategy with old vs new PM.

[lenary]

Add comments, remove unused code

lib/Transforms/IPO/WholeProgramDevirt.cpp

 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/BasicAliasAnalysis.h"
+#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
 #include "llvm/Analysis/TypeMetadataUtils.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/DerivedTypes.h"
-#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
struct VirtualCallSite {
   Value *VTable;
   CallSite CS;
 
   // If non-null, this field points to the associated unsafe use count stored in
   // the DevirtModule::NumUnsafeUsesForTypeTest map below. See the description
   // of that field for details.
   unsigned *NumUnsafeUses;
 
-  void emitRemark(const Twine &OptName, const Twine &TargetName) {
+  void
+  emitRemark(const StringRef OptName, const StringRef TargetName,
+             function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter) {
     Function *F = CS.getCaller();
-    emitOptimizationRemark(
-        F->getContext(), DEBUG_TYPE, *F,
-        CS.getInstruction()->getDebugLoc(),
-        OptName + ": devirtualized a call to " + TargetName);
-  }
+    DebugLoc DLoc = CS->getDebugLoc();
+    BasicBlock *Block = CS.getParent();
 
-  void replaceAndErase(const Twine &OptName, const Twine &TargetName,
-                       bool RemarksEnabled, Value *New) {
+    std::unique_ptr<OptimizationRemarkEmitter> OwnedORE;
+    OptimizationRemarkEmitter *ORE;
+    if (OREGetter)
+      ORE = &OREGetter(F);
+    else {
+      OwnedORE = make_unique<OptimizationRemarkEmitter>(F);
+      ORE = OwnedORE.get();
+    }
+
+    using namespace ore;
+    ORE->emit(OptimizationRemark(DEBUG_TYPE, OptName, DLoc, Block)
+              << NV("Optimization", OptName) << ": devirtualized a call to "
+              << NV("FunctionName", TargetName));
+  }
+
+  void replaceAndErase(
+      const StringRef OptName, const StringRef TargetName, bool RemarksEnabled,
+      function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter,
+      Value *New) {
     if (RemarksEnabled)
-      emitRemark(OptName, TargetName);
+      emitRemark(OptName, TargetName, OREGetter);
     CS->replaceAllUsesWith(New);
     if (auto II = dyn_cast<InvokeInst>(CS.getInstruction())) {
       BranchInst::Create(II->getNormalDest(), CS.getInstruction());
       II->getUnwindDest()->removePredecessor(II->getParent());
     }
     CS->eraseFromParent();
     // This use is no longer unsafe.
     if (NumUnsafeUses)
struct DevirtModule {
 
   IntegerType *Int8Ty;
   PointerType *Int8PtrTy;
   IntegerType *Int32Ty;
   IntegerType *Int64Ty;
   IntegerType *IntPtrTy;
 
   bool RemarksEnabled;
+  function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter;
 
   MapVector<VTableSlot, VTableSlotInfo> CallSlots;
 
   // This map keeps track of the number of "unsafe" uses of a loaded function
   // pointer. The key is the associated llvm.type.test intrinsic call generated
   // by this pass. An unsafe use is one that calls the loaded function pointer
   // directly. Every time we eliminate an unsafe use (for example, by
   // devirtualizing it or by applying virtual constant propagation), we
   // decrement the value stored in this map. If a value reaches zero, we can
   // eliminate the type check by RAUWing the associated llvm.type.test call with
   // true.
   std::map<CallInst *, unsigned> NumUnsafeUsesForTypeTest;
 
   DevirtModule(Module &M, function_ref<AAResults &(Function &)> AARGetter,
+               function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter,
                ModuleSummaryIndex *ExportSummary,
                const ModuleSummaryIndex *ImportSummary)
       : M(M), AARGetter(AARGetter), ExportSummary(ExportSummary),
         ImportSummary(ImportSummary), Int8Ty(Type::getInt8Ty(M.getContext())),
         Int8PtrTy(Type::getInt8PtrTy(M.getContext())),
         Int32Ty(Type::getInt32Ty(M.getContext())),
         Int64Ty(Type::getInt64Ty(M.getContext())),
         IntPtrTy(M.getDataLayout().getIntPtrType(M.getContext(), 0)),
-        RemarksEnabled(areRemarksEnabled()) {
+        RemarksEnabled(areRemarksEnabled()), OREGetter(OREGetter) {
     assert(!(ExportSummary && ImportSummary));
   }
 
   bool areRemarksEnabled();
 
   void scanTypeTestUsers(Function *TypeTestFunc, Function *AssumeFunc);
   void scanTypeCheckedLoadUsers(Function *TypeCheckedLoadFunc);
 
struct DevirtModule {
   // If we were able to eliminate all unsafe uses for a type checked load,
   // eliminate the associated type tests by replacing them with true.
   void removeRedundantTypeTests();
 
   bool run();
 
   // Lower the module using the action and summary passed as command line
   // arguments. For testing purposes only.
-  static bool runForTesting(Module &M,
-                            function_ref<AAResults &(Function &)> AARGetter);
+  static bool runForTesting(
+      Module &M, function_ref<AAResults &(Function &)> AARGetter,
+      function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter);
 };
 
 struct WholeProgramDevirt : public ModulePass {
   static char ID;
 
   bool UseCommandLine = false;
 
   ModuleSummaryIndex *ExportSummary;
   const ModuleSummaryIndex *ImportSummary;
 
   WholeProgramDevirt() : ModulePass(ID), UseCommandLine(true) {
     initializeWholeProgramDevirtPass(*PassRegistry::getPassRegistry());
   }
 
   WholeProgramDevirt(ModuleSummaryIndex *ExportSummary,
                      const ModuleSummaryIndex *ImportSummary)
       : ModulePass(ID), ExportSummary(ExportSummary),
         ImportSummary(ImportSummary) {
     initializeWholeProgramDevirtPass(*PassRegistry::getPassRegistry());
   }
 
   bool runOnModule(Module &M) override {
     if (skipModule(M))
       return false;
+
+    auto OREGetter = function_ref<OptimizationRemarkEmitter &(Function *)>();
+
     if (UseCommandLine)
-      return DevirtModule::runForTesting(M, LegacyAARGetter(*this));
-    return DevirtModule(M, LegacyAARGetter(*this), ExportSummary, ImportSummary)
+      return DevirtModule::runForTesting(M, LegacyAARGetter(*this), OREGetter);
+
+    return DevirtModule(M, LegacyAARGetter(*this), OREGetter, ExportSummary,
+                        ImportSummary)
         .run();
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<AssumptionCacheTracker>();
     AU.addRequired<TargetLibraryInfoWrapperPass>();
   }
 };

 }
 
 PreservedAnalyses WholeProgramDevirtPass::run(Module &M,
                                               ModuleAnalysisManager &AM) {
   auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
   auto AARGetter = [&](Function &F) -> AAResults & {
     return FAM.getResult<AAManager>(F);
   };
-  if (!DevirtModule(M, AARGetter, nullptr, nullptr).run())
+  auto OREGetter = [&](Function *F) -> OptimizationRemarkEmitter & {
+    return FAM.getResult<OptimizationRemarkEmitterAnalysis>(*F);
+  };
+  if (!DevirtModule(M, AARGetter, OREGetter, nullptr, nullptr).run())
     return PreservedAnalyses::all();
   return PreservedAnalyses::none();
 }
 
 bool DevirtModule::runForTesting(
-    Module &M, function_ref<AAResults &(Function &)> AARGetter) {
+    Module &M, function_ref<AAResults &(Function &)> AARGetter,
+    function_ref<OptimizationRemarkEmitter &(Function *)> OREGetter) {
   ModuleSummaryIndex Summary;
 
   // Handle the command-line summary arguments. This code is for testing
   // purposes only, so we handle errors directly.
   if (!ClReadSummary.empty()) {
     ExitOnError ExitOnErr("-wholeprogramdevirt-read-summary: " + ClReadSummary +
                           ": ");
     auto ReadSummaryFile =
         ExitOnErr(errorOrToExpected(MemoryBuffer::getFile(ClReadSummary)));
 
     yaml::Input In(ReadSummaryFile->getBuffer());
     In >> Summary;
     ExitOnErr(errorCodeToError(In.error()));
   }
 
   bool Changed =
       DevirtModule(
-          M, AARGetter,
+          M, AARGetter, OREGetter,
           ClSummaryAction == PassSummaryAction::Export ? &Summary : nullptr,
           ClSummaryAction == PassSummaryAction::Import ? &Summary : nullptr)
           .run();
 
   if (!ClWriteSummary.empty()) {
     ExitOnError ExitOnErr(
         "-wholeprogramdevirt-write-summary: " + ClWriteSummary + ": ");
     std::error_code EC;
bool DevirtModule::tryFindVirtualCallTargets(
   return !TargetsForSlot.empty();
 }
 
 void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo,
                                          Constant *TheFn, bool &IsExported) {
   auto Apply = [&](CallSiteInfo &CSInfo) {
     for (auto &&VCallSite : CSInfo.CallSites) {
       if (RemarksEnabled)
-        VCallSite.emitRemark("single-impl", TheFn->getName());
+        VCallSite.emitRemark("single-impl", TheFn->getName(), OREGetter);
       VCallSite.CS.setCalledFunction(ConstantExpr::getBitCast(
           TheFn, VCallSite.CS.getCalledValue()->getType()));
       // This use is no longer unsafe.
       if (VCallSite.NumUnsafeUses)
         --*VCallSite.NumUnsafeUses;
     }
     if (CSInfo.isExported()) {
       IsExported = true;
bool DevirtModule::tryEvaluateFunctionsWithArgs(
   }
   return true;
 }
 
 void DevirtModule::applyUniformRetValOpt(CallSiteInfo &CSInfo, StringRef FnName,
                                          uint64_t TheRetVal) {
   for (auto Call : CSInfo.CallSites)
     Call.replaceAndErase(
-        "uniform-ret-val", FnName, RemarksEnabled,
+        "uniform-ret-val", FnName, RemarksEnabled, OREGetter,
         ConstantInt::get(cast<IntegerType>(Call.CS.getType()), TheRetVal));
   CSInfo.markDevirt();
 }
 
 bool DevirtModule::tryUniformRetValOpt(
     MutableArrayRef<VirtualCallTarget> TargetsForSlot, CallSiteInfo &CSInfo,
     WholeProgramDevirtResolution::ByArg *Res) {
   // Uniform return value optimization. If all functions return the same

 void DevirtModule::applyUniqueRetValOpt(CallSiteInfo &CSInfo, StringRef FnName,
                                         bool IsOne,
                                         Constant *UniqueMemberAddr) {
   for (auto &&Call : CSInfo.CallSites) {
     IRBuilder<> B(Call.CS.getInstruction());
     Value *Cmp = B.CreateICmp(IsOne ? ICmpInst::ICMP_EQ : ICmpInst::ICMP_NE,
                               Call.VTable, UniqueMemberAddr);
     Cmp = B.CreateZExt(Cmp, Call.CS->getType());
-    Call.replaceAndErase("unique-ret-val", FnName, RemarksEnabled, Cmp);
+    Call.replaceAndErase("unique-ret-val", FnName, RemarksEnabled, OREGetter,
+                         Cmp);
   }
   CSInfo.markDevirt();
 }
 
 bool DevirtModule::tryUniqueRetValOpt(
     unsigned BitWidth, MutableArrayRef<VirtualCallTarget> TargetsForSlot,
     CallSiteInfo &CSInfo, WholeProgramDevirtResolution::ByArg *Res,
     VTableSlot Slot, ArrayRef<uint64_t> Args) {
for (auto Call : CSInfo.CallSites) {
     auto *RetType = cast<IntegerType>(Call.CS.getType());
     IRBuilder<> B(Call.CS.getInstruction());
     Value *Addr = B.CreateGEP(Int8Ty, Call.VTable, Byte);
     if (RetType->getBitWidth() == 1) {
       Value *Bits = B.CreateLoad(Addr);
       Value *BitsAndBit = B.CreateAnd(Bits, Bit);
       auto IsBitSet = B.CreateICmpNE(BitsAndBit, ConstantInt::get(Int8Ty, 0));
       Call.replaceAndErase("virtual-const-prop-1-bit", FnName, RemarksEnabled,
-                           IsBitSet);
+                           OREGetter, IsBitSet);
     } else {
       Value *ValAddr = B.CreateBitCast(Addr, RetType->getPointerTo());
       Value *Val = B.CreateLoad(RetType, ValAddr);
-      Call.replaceAndErase("virtual-const-prop", FnName, RemarksEnabled, Val);
+      Call.replaceAndErase("virtual-const-prop", FnName, RemarksEnabled,
+                           OREGetter, Val);
     }
   }
   CSInfo.markDevirt();
 }
 
 bool DevirtModule::tryVirtualConstProp(
     MutableArrayRef<VirtualCallTarget> TargetsForSlot, VTableSlotInfo &SlotInfo,
     WholeProgramDevirtResolution *Res, VTableSlot Slot) {
if (ExportSummary && isa<MDString>(S.first.TypeID)) {
           FS->addTypeTest(GUID);
     }
   }
 
   if (RemarksEnabled) {
     // Generate remarks for each devirtualized function.
     for (const auto &DT : DevirtTargets) {
       Function *F = DT.second;
-      DISubprogram *SP = F->getSubprogram();
-      emitOptimizationRemark(F->getContext(), DEBUG_TYPE, *F, SP,
-                             Twine("devirtualized ") + F->getName());
+      // DISubprogram *SP = F->getSubprogram();
+
+      std::unique_ptr<OptimizationRemarkEmitter> OwnedORE;
+      OptimizationRemarkEmitter *ORE;
+      if (OREGetter)
+        ORE = &OREGetter(F);
+      else {
+        OwnedORE = make_unique<OptimizationRemarkEmitter>(F);
+        ORE = OwnedORE.get();
+      }
+
+      using namespace ore;
+      ORE->emit(OptimizationRemark(DEBUG_TYPE, "Devirtualized", F)
+                << "devirtualized " << NV("FunctionName", F->getName()));
     }
   }
 
   removeRedundantTypeTests();
 
   // Rebuild each global we touched as part of virtual constant propagation to
   // include the before and after bytes.
   if (DidVirtualConstProp)
     for (VTableBits &B : Bits)
       rebuildGlobal(B);
 
   return true;
 }