[MachineInstr] Add isCandidateForCallSiteEntry predicate

[vsk]

Summary

Add the isCandidateForCallSiteEntry predicate to MachineInstr to
determine whether a DWARF call site entry should be created for an
instruction.

For now, it's enough to have any call instruction that doesn't belong to
a blacklisted set of opcodes. For these opcodes, a call site entry isn't
meaningful.

c.f. https://reviews.llvm.org/D74121, which takes a whitelisting approach.

[vsk]

• Add back IgnoreBundle.

[vsk]

I went ahead with the rebases for local testing, so I thought I'd share the patches here.

Rebased version of D73700:

0002-Move-call-site-info-where-previously-it-was-dropped-.patch5 KBDownload

Rebased version of D74122:

0003-Use-isCandidateForCallSiteEntry-when-updating-call-s.patch11 KBDownload

[djtodoro]

Hi @vsk, thanks for this!

If this defines all the instructions we want to avoid, this is better approach.

[vsk]

In D74159#1863457, @djtodoro wrote:

Hi @vsk, thanks for this!

If this defines all the instructions we want to avoid, this is better approach.

Yes, I believe this blacklist is complete for the currently supported set of targets. In the future, if we need to plumb target-specific information into this predicate, a TargetInstrInfo will be available via getMF()->getSubtarget().getInstrInfo().

llvm/include/llvm/CodeGen/MachineFunction.h

void setVariableDbgInfo(const DILocalVariable *Var, const DIExpression *Expr,
     VariableDbgInfos.emplace_back(Var, Expr, Slot, Loc);
   }
 
   VariableDbgInfoMapTy &getVariableDbgInfo() { return VariableDbgInfos; }
   const VariableDbgInfoMapTy &getVariableDbgInfo() const {
     return VariableDbgInfos;
   }
 
+  /// Start tracking the arguments passed to the call \p CallI.
   void addCallArgsForwardingRegs(const MachineInstr *CallI,
                                  CallSiteInfoImpl &&CallInfo) {
-    assert(CallI->isCall());
-    CallSitesInfo[CallI] = std::move(CallInfo);
+    assert(CallI->isCandidateForCallSiteEntry());
+    bool Inserted =
+        CallSitesInfo.try_emplace(CallI, std::move(CallInfo)).second;
+    (void)Inserted;
+    assert(Inserted && "Call site info not unique");
   }
 
   const CallSiteInfoMap &getCallSitesInfo() const {
     return CallSitesInfo;
   }
 
   /// Following functions update call site info. They should be called before
   /// removing, replacing or copying call instruction.

llvm/include/llvm/CodeGen/MachineInstr.h

public:
   bool isEHScopeReturn(QueryType Type = AnyInBundle) const {
     return hasProperty(MCID::EHScopeReturn, Type);
   }
 
   bool isCall(QueryType Type = AnyInBundle) const {
     return hasProperty(MCID::Call, Type);
   }
 
+  /// Return true if this is a call instruction that may have an associated
+  /// call site entry in the debug info.
+  bool isCandidateForCallSiteEntry() const;
+
   /// Returns true if the specified instruction stops control flow
   /// from executing the instruction immediately following it.  Examples include
   /// unconditional branches and return instructions.
   bool isBarrier(QueryType Type = AnyInBundle) const {
     return hasProperty(MCID::Barrier, Type);
   }
 
   /// Returns true if this instruction part of the terminator for a basic block.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

for (const MachineInstr &MI : MBB.instrs()) {
       // Bundles with call in them will pass the isCall() test below but do not
       // have callee operand information so skip them here. Iterator will
       // eventually reach the call MI.
       if (MI.isBundle())
         continue;
 
       // Skip instructions which aren't calls. Both calls and tail-calling jump
       // instructions (e.g TAILJMPd64) are classified correctly here.
-      if (!MI.isCall())
+      if (!MI.isCandidateForCallSiteEntry())
         continue;
 
       // TODO: Add support for targets with delay slots (see: beginInstruction).
       if (MI.hasDelaySlot())
         return;
 
       // If this is a direct call, find the callee's subprogram.
       // In the case of an indirect call find the register that holds

llvm/lib/CodeGen/MachineInstr.cpp

void MachineInstr::eraseFromParentAndMarkDBGValuesForRemoval() {
   MI->eraseFromParent();
 }
 
 void MachineInstr::eraseFromBundle() {
   assert(getParent() && "Not embedded in a basic block!");
   getParent()->erase_instr(this);
 }
 
+bool MachineInstr::isCandidateForCallSiteEntry() const {
+  if (!isCall(MachineInstr::IgnoreBundle))
+    return false;
+  switch (getOpcode()) {
+  case TargetOpcode::PATCHABLE_EVENT_CALL:
+  case TargetOpcode::PATCHABLE_TYPED_EVENT_CALL:
+  case TargetOpcode::PATCHPOINT:
+  case TargetOpcode::STACKMAP:
+  case TargetOpcode::STATEPOINT:
+    return false;
+  }
+  return true;
+}
+
 unsigned MachineInstr::getNumExplicitOperands() const {
   unsigned NumOperands = MCID->getNumOperands();
   if (!MCID->isVariadic())
     return NumOperands;
 
   for (unsigned I = NumOperands, E = getNumOperands(); I != E; ++I) {
     const MachineOperand &MO = getOperand(I);
     // The operands must always be in the following order:

llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp

if (Before == BB->end()) {
       // There were no prior instructions; the new ones must start at the
       // beginning of the block.
       MI = &Emitter.getBlock()->instr_front();
     } else {
       // Return first instruction after the pre-existing instructions.
       MI = &*std::next(Before);
     }
 
-    if (MI->isCall() && DAG->getTarget().Options.EnableDebugEntryValues)
+    if (MI->isCandidateForCallSiteEntry() &&
+        DAG->getTarget().Options.EnableDebugEntryValues)
       MF.addCallArgsForwardingRegs(MI, DAG->getSDCallSiteInfo(Node));
 
     return MI;
   };
 
   // If this is the first BB, emit byval parameter dbg_value's.
   if (HasDbg && BB->getParent()->begin() == MachineFunction::iterator(BB)) {
     SDDbgInfo::DbgIterator PDI = DAG->ByvalParmDbgBegin();

llvm/test/CodeGen/AArch64/arm64-anyregcc.ll

-; RUN: llc < %s -mtriple=arm64-apple-darwin | FileCheck %s
+; RUN: llc < %s -debug-entry-values -mtriple=arm64-apple-darwin | FileCheck %s
 
 ; Stackmap Header: no constants - 6 callsites
 ; CHECK-LABEL: .section __LLVM_STACKMAPS,__llvm_stackmaps
 ; CHECK-NEXT:  __LLVM_StackMaps:
 ; Header
 ; CHECK-NEXT:   .byte 3
 ; CHECK-NEXT:   .byte 0
 ; CHECK-NEXT:   .short 0

llvm/test/CodeGen/AArch64/arm64-patchpoint.ll

-; RUN: llc -mtriple=arm64-apple-darwin -enable-misched=0 -mcpu=cyclone                             < %s | FileCheck %s
-; RUN: llc -mtriple=arm64-apple-darwin -enable-misched=0 -mcpu=cyclone -fast-isel -fast-isel-abort=1 < %s | FileCheck %s
+; RUN: llc -mtriple=arm64-apple-darwin -debug-entry-values -enable-misched=0 -mcpu=cyclone                             < %s | FileCheck %s
+; RUN: llc -mtriple=arm64-apple-darwin -debug-entry-values -enable-misched=0 -mcpu=cyclone -fast-isel -fast-isel-abort=1 < %s | FileCheck %s
 
 ; Trivial patchpoint codegen
 ;
 define i64 @trivial_patchpoint_codegen(i64 %p1, i64 %p2, i64 %p3, i64 %p4) {
 entry:
 ; CHECK-LABEL: trivial_patchpoint_codegen:
 ; CHECK:       mov  x16, #244834610708480
 ; CHECK-NEXT:  movk x16, #48879, lsl #16

llvm/test/CodeGen/X86/statepoint-allocas.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -debug-entry-values < %s | FileCheck %s
 ; Check that we can lower a use of an alloca both as a deopt value (where the
 ; exact meaning is up to the consumer of the stackmap) and as an explicit spill
 ; slot used for GC.
 
 target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 declare zeroext i1 @return_i1()

llvm/test/CodeGen/X86/xray-custom-log.ll

-; RUN: llc -verify-machineinstrs -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
-; RUN: llc -verify-machineinstrs -filetype=asm -o - \
+; RUN: llc -verify-machineinstrs -debug-entry-values -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -debug-entry-values -filetype=asm -o - \
 ; RUN:     -mtriple=x86_64-unknown-linux-gnu -relocation-model=pic < %s | FileCheck %s -check-prefix=PIC
 
 define i32 @fn() nounwind noinline uwtable "function-instrument"="xray-always" {
     %eventptr = alloca i8
     %eventsize = alloca i32
     store i32 3, i32* %eventsize
     %val = load i32, i32* %eventsize
     call void @llvm.xray.customevent(i8* %eventptr, i32 %val)

llvm/test/CodeGen/X86/xray-typed-event-log.ll

-; RUN: llc -verify-machineinstrs -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
-; RUN: llc -verify-machineinstrs -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu \
+; RUN: llc -verify-machineinstrs -debug-entry-values -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -debug-entry-values -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu \
 ; RUN:    -relocation-model=pic < %s | FileCheck %s -check-prefix=PIC
 
 define i32 @fn() nounwind noinline uwtable "function-instrument"="xray-always" {
     %eventptr = alloca i8
     %eventsize = alloca i32
     %eventtype = alloca i16
     store i16 6, i16* %eventtype
     %type = load i16, i16* %eventtype