OpenMPOpt Remarks Support

[jhuber6]

Summary

Adding initial support for emitting remarks inside of OpenMPOpt. Added a Callback function to get an OptimizationRemarkEmitter object to the OpenMPOpt struct. The callback is used by the emitRemark function. Added two locations that emit simple remarks and a test file to verify functionality.

[jdoerfert]

Looks pretty good. Some minor comments

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

508

Nit: getFunction()

auto & or auto *, please.

Do you need a default for the callback type?

558

I don't think it's a good idea to move the result here. Just return a reference.

609

Call getAnalysis here?

[jhuber6]

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

508

I changed the OREGetter to return a reference. I'm not exactly sure what you mean by default for the callback type. Do you mean a default implementation or are you talking about just the template parameter.

558

Yeah, I did this initially because I was having issues getting both the legacy and new versions to return a reference without memory leaks so I just did it by returning a unique_ptr. The std::move just cast it to an rvalue to avoid using the deleted constructor in the ORE class. I found another file that did something similar that had a single unique_ptr object to an ORE that's captured by reference. Then you just return a reference to that, so I'll change it to use that method because it's probably much more efficient.

609

I tried using the getAnalysis method but it caused it to seg-fault. I'm not exactly confident why but some other passes generate the ORE this way so I figured it was okay.

[jhuber6]

Changed OREGetter to return the ORE by reference rather than with a unique_ptr.

[jdoerfert]

Please diff against the base version, e.g., by squashing all your commits into a single one.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

508

The latter. Doesn't this work:

template <typename RemarkKind, typename RemarkCallBack>

609

Oh, right. Yeah, so, the old PM CGSCC pass is bad at requiring function passes. Long story short, it doesn't work.

We should do what the inliner does, create an ORE whenever we need one for a function. Since we would also like hotness information in the new pass manager (which I think we should get automatically via the pass), we should not always create a new ORE but only in the old PM. That said, the ORE constructor says it can be expensive if hotness filtering is requested as block frequency is computed. Thus, in the old PM we can cache the OREs ourselves. So, we keep a map from function to ORE and in the OREGetter we lookup the ORE, if not there create it, and return it. WDYT?

[jhuber6]

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

508

I was just using that template to rename the type inside the function. I was pretty much just using it as this:

using RemarkCallBack = function_ref<RemarkKind(RemarkKind &&)>>;

Should I change that?

609

I was thinking about adding a cache previously since the function being worked on might not change often. It might be somewhat difficult to bake that functionality into the callback but I'll see what I can do in the morning.

[jhuber6]

I added a DenseMap to cache the Functions for the legacy PM's getter function. Let me know if I should've used a different data structure for this.

[jhuber6]

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

609

I just noticed I could probably have used try_emplace() rather than checking and inserting manually.

[jdoerfert]

Last round of comments. After we should be good to go.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

30

Nit: probably not needed.

61

Just copy the getter (=function_ref)

508

I don't get the last comment but I guess we can just leave it as is.

613

I usually prefer:

std::make_unique<OptimizationRemarkEmitter> &ORE = OREMap[F];
if(!ORE)
  ORE = ...
return ORE;

Your code has 3 lookups into the map, mine only one.

[jhuber6]

Making suggested changes.

[jdoerfert]

LGTM. I try to commit it later.

[hfinkel]

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

288

Why doesn't RFI.Name have a ore::NV field name. Tools might certainly be interested in what functions are being optimized.

321

Same here: a NV for RFI.Name seems useful.

[jhuber6]

Adding a remark and test for parallel region deletion. Added a NV tag for the runtime function deduplicated / moved.

[jhuber6]

Just realized the master branch changed the attributor flags for the test file. Should I add a diff that's more up-to-date?

[jhuber6]

Updating files closer to origin.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/IPO/OpenMPOpt.h"
 
 #include "llvm/ADT/EnumeratedArray.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/Analysis/CallGraphSCCPass.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/Utils/CallGraphUpdater.h"
 
 using namespace llvm;
 using namespace omp;
 using namespace types;
 
 #define DEBUG_TYPE "openmp-opt"
 
 static cl::opt<bool> DisableOpenMPOptimizations(
     "openmp-opt-disable", cl::ZeroOrMore,
     cl::desc("Disable OpenMP specific optimizations."), cl::Hidden,
     cl::init(false));
 
 STATISTIC(NumOpenMPRuntimeCallsDeduplicated,
           "Number of OpenMP runtime calls deduplicated");
 STATISTIC(NumOpenMPRuntimeFunctionsIdentified,
           "Number of OpenMP runtime functions identified");
 STATISTIC(NumOpenMPRuntimeFunctionUsesIdentified,
           "Number of OpenMP runtime function uses identified");
 
 #if !defined(NDEBUG)
 static constexpr auto TAG = "[" DEBUG_TYPE "]";
 #endif
 
 namespace {
 struct OpenMPOpt {
 
+  using OptimizationRemarkGetter =
+      function_ref<OptimizationRemarkEmitter &(Function *)>;
+
   OpenMPOpt(SmallPtrSetImpl<Function *> &SCC,
             SmallPtrSetImpl<Function *> &ModuleSlice,
-            CallGraphUpdater &CGUpdater)
+            CallGraphUpdater &CGUpdater, OptimizationRemarkGetter OREGetter)
       : M(*(*SCC.begin())->getParent()), SCC(SCC), ModuleSlice(ModuleSlice),
-        OMPBuilder(M), CGUpdater(CGUpdater) {
+        OMPBuilder(M), CGUpdater(CGUpdater), OREGetter(OREGetter) {
     initializeTypes(M);
     initializeRuntimeFunctions();
     OMPBuilder.initialize();
   }
 
   /// Generic information that describes a runtime function
   struct RuntimeFunctionInfo {
     /// The kind, as described by the RuntimeFunction enum.
auto CanBeMoved = [](CallBase &CB) {
       return true;
     };
 
     if (!ReplVal) {
       for (Use *U : Uses)
         if (CallInst *CI = getCallIfRegularCall(*U, &RFI)) {
           if (!CanBeMoved(*CI))
             continue;
+
+          auto Remark = [&](OptimizationRemark OR) {
+            auto newLoc = &*F.getEntryBlock().getFirstInsertionPt();
+            return OR << "OpenMP runtime call " << RFI.Name << " moved to "
+                      << ore::NV("OpenMPRuntimeMoves", newLoc->getDebugLoc());
+          };
+          emitRemark<OptimizationRemark>(CI, "OpenMPRuntimeCodeMotion", Remark);
+
           CI->moveBefore(&*F.getEntryBlock().getFirstInsertionPt());
           ReplVal = CI;
           break;
         }
       if (!ReplVal)
         return false;
     }
 
bool deduplicateRuntimeCalls(Function &F, RuntimeFunctionInfo &RFI,
     }
 
     bool Changed = false;
     auto ReplaceAndDeleteCB = [&](Use &U, Function &Caller) {
       CallInst *CI = getCallIfRegularCall(U, &RFI);
       if (!CI || CI == ReplVal || &F != &Caller)
         return false;
       assert(CI->getCaller() == &F && "Unexpected call!");
+
+      auto Remark = [&](OptimizationRemark OR) {
+        return OR << "OpenMP runtime call " << RFI.Name << " deduplicated";
+      };
+      emitRemark<OptimizationRemark>(CI, "OpenMPRuntimeDeduplicated", Remark);
+
       CGUpdater.removeCallSite(*CI);
       CI->replaceAllUsesWith(ReplVal);
       CI->eraseFromParent();
       ++NumOpenMPRuntimeCallsDeduplicated;
       Changed = true;
       return true;
     };
     RFI.foreachUse(ReplaceAndDeleteCB);
if (declMatchesRTFTypes(F, _ReturnType , ArgsTypes)) {                     \
       });                                                                      \
     }                                                                          \
   }
 #include "llvm/Frontend/OpenMP/OMPKinds.def"
 
     // TODO: We should attach the attributes defined in OMPKinds.def.
   }
 
+  /// Emit a remark generically
+  ///
+  /// This template function can be used to generically emit a remark. The
+  /// RemarkKind should be one of the following:
+  ///   - OptimizationRemark to indicate a successful optimization attempt
+  ///   - OptimizationRemarkMissed to report a failed optimization attempt
+  ///   - OptimizationRemarkAnalysis to provide additional information about an
+  ///     optimization attempt
+  ///
+  /// The remark is built using a callback function provided by the caller that
+  /// takes a RemarkKind as input and returns a RemarkKind.
+  template <typename RemarkKind,
+            typename RemarkCallBack = function_ref<RemarkKind(RemarkKind &&)>>
+  void emitRemark(Instruction *Inst, StringRef RemarkName,
+                  RemarkCallBack &&RemarkCB) {
+    Function *F = Inst->getParent()->getParent();
+    auto &ORE = OREGetter(F);
+
+    ORE.emit([&]() {
+      return RemarkCB(RemarkKind(DEBUG_TYPE, RemarkName, Inst));
+    });
+  }
+
   /// The underyling module.
   Module &M;
 
   /// The SCC we are operating on.
   SmallPtrSetImpl<Function *> &SCC;
 
   /// The slice of the module we are allowed to look at.
   SmallPtrSetImpl<Function *> &ModuleSlice;
 
   /// An OpenMP-IR-Builder instance
   OpenMPIRBuilder OMPBuilder;
 
   /// Callback to update the call graph, the first argument is a removed call,
   /// the second an optional replacement call.
   CallGraphUpdater &CGUpdater;
 
+  /// Callback to get an OptimizationRemarkEmitter from a Function *
+  OptimizationRemarkGetter OREGetter;
+
   /// Map from runtime function kind to the runtime function description.
   EnumeratedArray<RuntimeFunctionInfo, RuntimeFunction,
                   RuntimeFunction::OMPRTL___last>
       RFIs;
 };
 } // namespace
 
 PreservedAnalyses OpenMPOptPass::run(LazyCallGraph::SCC &C,
                                      CGSCCAnalysisManager &AM,
                                      LazyCallGraph &CG, CGSCCUpdateResult &UR) {
   if (!containsOpenMP(*C.begin()->getFunction().getParent(), OMPInModule))
     return PreservedAnalyses::all();
 
   if (DisableOpenMPOptimizations)
     return PreservedAnalyses::all();
 
   SmallPtrSet<Function *, 16> SCC;
   for (LazyCallGraph::Node &N : C)
     SCC.insert(&N.getFunction());
 
   if (SCC.empty())
     return PreservedAnalyses::all();
 
+  auto OREGetter = [&C, &CG, &AM](Function *F) -> OptimizationRemarkEmitter & {
+    FunctionAnalysisManager &FAM =
+        AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, CG).getManager();
+    return FAM.getResult<OptimizationRemarkEmitterAnalysis>(*F);
+  };
+
   CallGraphUpdater CGUpdater;
   CGUpdater.initialize(CG, C, AM, UR);
   // TODO: Compute the module slice we are allowed to look at.
-  OpenMPOpt OMPOpt(SCC, SCC, CGUpdater);
+  OpenMPOpt OMPOpt(SCC, SCC, CGUpdater, OREGetter);
   bool Changed = OMPOpt.run();
   (void)Changed;
   return PreservedAnalyses::all();
 }
 
 namespace {
 
 struct OpenMPOptLegacyPass : public CallGraphSCCPass {
for (CallGraphNode *CGN : CGSCC)
           SCC.insert(Fn);
 
     if (SCC.empty())
       return false;
 
     CallGraph &CG = getAnalysis<CallGraphWrapperPass>().getCallGraph();
     CGUpdater.initialize(CG, CGSCC);
 
+    // Maintain a map of functions to avoid recomputing the ORE
+    DenseMap<Function *, std::unique_ptr<OptimizationRemarkEmitter>> OREMap;
+    auto OREGetter = [&OREMap](Function *F) -> OptimizationRemarkEmitter & {
+      std::unique_ptr<OptimizationRemarkEmitter> &ORE = OREMap[F];
+      if (!ORE)
+        ORE = std::make_unique<OptimizationRemarkEmitter>(F);
+      return *ORE;
+    };
+
     // TODO: Compute the module slice we are allowed to look at.
-    OpenMPOpt OMPOpt(SCC, SCC, CGUpdater);
+    OpenMPOpt OMPOpt(SCC, SCC, CGUpdater, OREGetter);
     return OMPOpt.run();
   }
 
   bool doFinalization(CallGraph &CG) override { return CGUpdater.finalize(); }
 };
 
 } // end anonymous namespace
 

llvm/test/Transforms/OpenMP/deduplication_remarks.ll

+; RUN: opt -openmpopt -pass-remarks=openmp-opt -disable-output < %s 2>&1 | FileCheck %s
+; RUN: opt -passes=openmpopt -pass-remarks=openmp-opt -disable-output < %s 2>&1 | FileCheck %s
+; ModuleID = 'deduplication_remarks.c'
+source_filename = "deduplication_remarks.c"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+%struct.ident_t = type { i32, i32, i32, i32, i8* }
+
+@0 = private unnamed_addr global %struct.ident_t { i32 0, i32 34, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str0, i32 0, i32 0) }, align 8
+@.str0 = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
+
+; CHECK: remark: deduplication_remarks.c:9:10: OpenMP runtime call __kmpc_global_thread_num moved to deduplication_remarks.c:5:10
+; CHECK: remark: deduplication_remarks.c:7:10: OpenMP runtime call __kmpc_global_thread_num deduplicated
+; CHECK: remark: deduplication_remarks.c:5:10: OpenMP runtime call __kmpc_global_thread_num deduplicated
+define dso_local void @deduplicate() local_unnamed_addr !dbg !14 {
+  %1 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0), !dbg !21
+  call void @useI32(i32 %1), !dbg !23
+  %2 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0), !dbg !24
+  call void @useI32(i32 %2), !dbg !25
+  %3 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0), !dbg !26
+  call void @useI32(i32 %3), !dbg !27
+  ret void, !dbg !28
+}
+
+declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
+
+declare !dbg !4 void @useI32(i32) local_unnamed_addr
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!8, !9, !10, !11, !12}
+!llvm.ident = !{!13}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "deduplication_remarks.c", directory: "/tmp")
+!2 = !{}
+!3 = !{!4}
+!4 = !DISubprogram(name: "useI32", scope: !1, file: !1, line: 1, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+!5 = !DISubroutineType(types: !6)
+!6 = !{null, !7}
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !{i32 7, !"Dwarf Version", i32 4}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 4}
+!11 = !{i32 7, !"PIC Level", i32 2}
+!12 = !{i32 7, !"PIE Level", i32 2}
+!13 = !{!"clang version 10.0.0 "}
+!14 = distinct !DISubprogram(name: "deduplicate", scope: !1, file: !1, line: 4, type: !15, scopeLine: 4, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !17)
+!15 = !DISubroutineType(types: !16)
+!16 = !{null}
+!17 = !{!18, !19, !20}
+!18 = !DILocalVariable(name: "x", scope: !14, file: !1, line: 5, type: !7)
+!19 = !DILocalVariable(name: "y", scope: !14, file: !1, line: 7, type: !7)
+!20 = !DILocalVariable(name: "z", scope: !14, file: !1, line: 9, type: !7)
+!21 = !DILocation(line: 5, column: 10, scope: !14)
+!22 = !DILocation(line: 0, scope: !14)
+!23 = !DILocation(line: 6, column: 2, scope: !14)
+!24 = !DILocation(line: 7, column: 10, scope: !14)
+!25 = !DILocation(line: 8, column: 2, scope: !14)
+!26 = !DILocation(line: 9, column: 10, scope: !14)
+!27 = !DILocation(line: 10, column: 2, scope: !14)
+!28 = !DILocation(line: 13, column: 1, scope: !14)