[llvm] Opt-in flag for X86DiscriminateMemOps

[mtrofin]

Summary

Currently, if an instruction with a memory operand has no debug information, X86DiscriminateMemOps will generate one based on the first line of the enclosing function, or the last seen debug info.

This may cause confusion in certain debugging scenarios. The long term approach would be to use the line number '0' in such cases, however, that brings in challenges: the base discriminator value range is limited (4096 values).

For the short term, adding an opt-in flag for this feature.

See bug 40319 (https://bugs.llvm.org/show_bug.cgi?id=40319)

[mtrofin]

git commit message update

[dblaikie]

Looks good - thanks!

[lebedev.ri]

This review happened without -commits list being subscribed.
(can that be fixes please, so many reviews recently bypass lists, it's raising questions)

[dblaikie]

In D57257#1371877, @lebedev.ri wrote:

This review happened without -commits list being subscribed.
(can that be fixes please, so many reviews recently bypass lists, it's raising questions)

@klimek - Manuel, any idea why this is? I thought generally this had been fixed for a while now.

[lebedev.ri]

In D57257#1371885, @dblaikie wrote:

In D57257#1371877, @lebedev.ri wrote:

This review happened without -commits list being subscribed.
(can that be fixes please, so many reviews recently bypass lists, it's raising questions)

@klimek - Manuel, any idea why this is? I thought generally this had been fixed for a while now.

Ugh, i *just* submitted a mail about this, let me add more CC's.

llvm/lib/Target/X86/X86DiscriminateMemOps.cpp

 #include "llvm/ProfileData/SampleProf.h"
 #include "llvm/ProfileData/SampleProfReader.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/IPO/SampleProfile.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "x86-discriminate-memops"
 
+static cl::opt<bool> EnableDiscriminateMemops(
+    DEBUG_TYPE, cl::init(false),
+    cl::desc("Generate unique debug info for each instruction with a memory "
+             "operand. Should be enabled for profile-drived cache prefetching, "
+             "both in the build of the binary being profiled, as well as in "
+             "the build of the binary consuming the profile."),
+    cl::Hidden);
+
 namespace {
 
 using Location = std::pair<StringRef, unsigned>;
 
 Location diToLocation(const DILocation *Loc) {
   return std::make_pair(Loc->getFilename(), Loc->getLine());
 }
 

 //===----------------------------------------------------------------------===//
 
 char X86DiscriminateMemOps::ID = 0;
 
 /// Default construct and initialize the pass.
 X86DiscriminateMemOps::X86DiscriminateMemOps() : MachineFunctionPass(ID) {}
 
 bool X86DiscriminateMemOps::runOnMachineFunction(MachineFunction &MF) {
+  if (!EnableDiscriminateMemops)
+    return false;
+
   DISubprogram *FDI = MF.getFunction().getSubprogram();
   if (!FDI || !FDI->getUnit()->getDebugInfoForProfiling())
     return false;
 
   // Have a default DILocation, if we find instructions with memops that don't
   // have any debug info.
   const DILocation *ReferenceDI =
       DILocation::get(FDI->getContext(), FDI->getLine(), 0, FDI);

llvm/lib/Target/X86/X86InsertPrefetch.cpp

 #include "llvm/ProfileData/SampleProf.h"
 #include "llvm/ProfileData/SampleProfReader.h"
 #include "llvm/Transforms/IPO/SampleProfile.h"
 using namespace llvm;
 using namespace sampleprof;
 
 static cl::opt<std::string>
     PrefetchHintsFile("prefetch-hints-file",
-                      cl::desc("Path to the prefetch hints profile."),
+                      cl::desc("Path to the prefetch hints profile. See also "
+                               "-x86-discriminate-memops"),
                       cl::Hidden);
 namespace {
 
 class X86InsertPrefetch : public MachineFunctionPass {
   void getAnalysisUsage(AnalysisUsage &AU) const override;
   bool doInitialization(Module &) override;
 
   bool runOnMachineFunction(MachineFunction &MF) override;

llvm/test/CodeGen/X86/discriminate-mem-ops.ll

-; RUN: llc < %s | FileCheck %s
+; RUN: llc -x86-discriminate-memops  < %s | FileCheck %s
 ;
 ; original source, compiled with -O3 -gmlt -fdebug-info-for-profiling:
 ; int sum(int* arr, int pos1, int pos2) {
 ;   return arr[pos1] + arr[pos2];
 ; }
 ;
 ; ModuleID = 'test.cc'
 source_filename = "test.cc"

llvm/test/CodeGen/X86/insert-prefetch-inline.ll

-; RUN: llc < %s -prefetch-hints-file=%S/insert-prefetch-inline.afdo | FileCheck %s
+; RUN: llc < %s -x86-discriminate-memops -prefetch-hints-file=%S/insert-prefetch-inline.afdo | FileCheck %s
 ;
 ; Verify we can insert prefetch instructions in code belonging to inlined
 ; functions.
 ;
 ; ModuleID = 'test.cc'
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

llvm/test/CodeGen/X86/insert-prefetch-invalid-instr.ll

-; RUN: llc < %s -prefetch-hints-file=%S/insert-prefetch-invalid-instr.afdo | FileCheck %s
+; RUN: llc < %s -x86-discriminate-memops -prefetch-hints-file=%S/insert-prefetch-invalid-instr.afdo | FileCheck %s
 ; ModuleID = 'prefetch.cc'
 source_filename = "prefetch.cc"
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: norecurse nounwind uwtable
 define dso_local i32 @main() local_unnamed_addr #0 !dbg !7 {
 entry:

llvm/test/CodeGen/X86/insert-prefetch.ll

-; RUN: llc < %s -prefetch-hints-file=%S/insert-prefetch.afdo | FileCheck %s
-; RUN: llc < %s -prefetch-hints-file=%S/insert-prefetch-other.afdo | FileCheck %s -check-prefix=OTHERS
+; RUN: llc < %s -x86-discriminate-memops -prefetch-hints-file=%S/insert-prefetch.afdo | FileCheck %s
+; RUN: llc < %s -x86-discriminate-memops -prefetch-hints-file=%S/insert-prefetch-other.afdo | FileCheck %s -check-prefix=OTHERS
 ;
 ; original source, compiled with -O3 -gmlt -fdebug-info-for-profiling:
 ; int sum(int* arr, int pos1, int pos2) {
 ;   return arr[pos1] + arr[pos2];
 ; }
 ;
 ; NOTE: debug line numbers were adjusted such that the function would start
 ; at line 15 (an arbitrary number). The sample profile file format uses