[SelectionDAG][NFCI] Use common logic for identifying MMI vars

[fdeazeve]

Summary

After function argument lowering, but prior to instruction selection,
dbg declares pointing to function arguments are lowered using special
logic.

Later, during instruction selection (both "fast" and regular ISel), this
logic is "repeated" in order to identify which intrinsics have already
been lowered. This is bad for two reasons:

1. The logic is not _really_ repeated, the code is different, which

could lead to duplicate lowering of the intrinsic.

2. Even if the logic were repeated properly, this is still code

duplication.

This patch addresses these issues by storing all preprocessed
dbg.declare intrinsics in a set inside FuncInfo; the set is queried upon
instruction selection.

[fdeazeve]

Update some code comments

[fdeazeve]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

6174

In case you're curious, all of this code is duplicated in the next file (SelectionDAGISel.cpp). You can see it in this review.

Note how the FastIsel branch had a very different logic (see the previous file in this review), even though it also calls this deleted code.

[aprantl]

This looks like a nice simplification!

llvm/include/llvm/CodeGen/FunctionLoweringInfo.h

192

///

193

4 seems like a low number. You could hack up a version of clang to dump the number of elements at the end and then pick something close to the median. Or you could pick 8 :-)

[Orlando]

Nice, that's much better!

(This is just a passing thumbs up, FWIW it LGTM too)

[fdeazeve]

llvm/include/llvm/CodeGen/FunctionLoweringInfo.h

193

(Side note, I found out about the st --summary CLI tool, which is very useful)

Hah! This actually revealed a problem with the patch :)

Because this class doesn't rely on RAII (we keep reusing it for all functions in the Module), it has a "clear" method that needs to be manually updated. When I tried compiling a big-ish cpp file, the size of the set kept growing and growing. I've now fixed this.

I compiled APInt.cpp with this change printing the set size after each function is lowered:

min     q1      median  q3      max
0       0       0       0       17       # O2
0       1       2       4       29       # O0

When compiling SelectionDAGISel.cpp, we also get similar ideas:

min     q1      median  q3      max
0       0       0       0       15       # O2
0       1       1       2       258      # O0

Since one of the Q3 == 4, and eyeballing the numbers there was a decent chunk of values in the [5;8] range, I'll just go ahead and use 8 to make the vast majority of cases fall in the "Small" case.

[fdeazeve]

Addressed review comments.

llvm/include/llvm/CodeGen/FunctionLoweringInfo.h

 #include <utility>
 #include <vector>
 
 namespace llvm {
 
 class Argument;
 class BasicBlock;
 class BranchProbabilityInfo;
+class DbgDeclareInst;
 class Function;
 class Instruction;
 class MachineFunction;
 class MachineInstr;
 class MachineRegisterInfo;
 class MVT;
 class SelectionDAG;
 class TargetLowering;
public:
   std::vector<std::pair<MachineInstr*, unsigned> > PHINodesToUpdate;
   unsigned OrigNumPHINodesToUpdate;
 
   /// If the current MBB is a landing pad, the exception pointer and exception
   /// selector registers are copied into these virtual registers by
   /// SelectionDAGISel::PrepareEHLandingPad().
   unsigned ExceptionPointerVirtReg, ExceptionSelectorVirtReg;
 
+  /// Collection of dbg.declare instructions handled after argument
+  /// lowering and before ISel proper.
+  SmallPtrSet<const DbgDeclareInst *, 8> PreprocessedDbgDeclares;
+
   /// set - Initialize this FunctionLoweringInfo with the given Function
   /// and its associated MachineFunction.
   ///
   void set(const Function &Fn, MachineFunction &MF, SelectionDAG *DAG);
 
   /// clear - Clear out all the function-specific state. This returns this
   /// FunctionLoweringInfo to an empty state, ready to be used for a
   /// different function.

llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

case Intrinsic::dbg_declare: {
     const DbgDeclareInst *DI = cast<DbgDeclareInst>(II);
     assert(DI->getVariable() && "Missing variable");
     if (!FuncInfo.MF->getMMI().hasDebugInfo()) {
       LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI
                         << " (!hasDebugInfo)\n");
       return true;
     }
 
+    if (FuncInfo.PreprocessedDbgDeclares.contains(DI))
+      return true;
+
     const Value *Address = DI->getAddress();
     if (!Address || isa<UndefValue>(Address)) {
       LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI
                         << " (bad/undef address)\n");
       return true;
     }
 
-    // Byval arguments with frame indices were already handled after argument
-    // lowering and before isel.
-    const auto *Arg =
-        dyn_cast<Argument>(Address->stripInBoundsConstantOffsets());
-    if (Arg && FuncInfo.getArgumentFrameIndex(Arg) != INT_MAX)
-      return true;
-
     std::optional<MachineOperand> Op;
     if (Register Reg = lookUpRegForValue(Address))
       Op = MachineOperand::CreateReg(Reg, false);
 
     // If we have a VLA that has a "use" in a metadata node that's then used
     // here but it has no other uses, then we have a problem. E.g.,
     //
     //   int foo (const int *x) {

llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp

void FunctionLoweringInfo::clear() {
   ArgDbgValues.clear();
   DescribedArgs.clear();
   ByValArgFrameIndexMap.clear();
   RegFixups.clear();
   RegsWithFixups.clear();
   StatepointStackSlots.clear();
   StatepointRelocationMaps.clear();
   PreferredExtendType.clear();
+  PreprocessedDbgDeclares.clear();
 }
 
 /// CreateReg - Allocate a single virtual register for the given type.
 Register FunctionLoweringInfo::CreateReg(MVT VT, bool isDivergent) {
   return RegInfo->createVirtualRegister(TLI->getRegClassFor(VT, isDivergent));
 }
 
 /// CreateRegs - Allocate the appropriate number of virtual registers of

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

case Intrinsic::call_preallocated_arg: {
     SDValue Res = DAG.getNode(
         ISD::PREALLOCATED_ARG, sdl,
         DAG.getVTList(TLI.getPointerTy(DAG.getDataLayout()), MVT::Other), Ops);
     setValue(&I, Res);
     DAG.setRoot(Res.getValue(1));
     return;
   }
   case Intrinsic::dbg_declare: {
+    const auto &DI = cast<DbgDeclareInst>(I);
     // Debug intrinsics are handled separately in assignment tracking mode.
-    if (AssignmentTrackingEnabled)
+    // Some intrinsics are handled right after Argument lowering.
+    if (AssignmentTrackingEnabled ||
+        FuncInfo.PreprocessedDbgDeclares.count(&DI))
       return;
     // Assume dbg.declare can not currently use DIArgList, i.e.
     // it is non-variadic.
-    const auto &DI = cast<DbgVariableIntrinsic>(I);
     assert(!DI.hasArgList() && "Only dbg.value should currently use DIArgList");
     DILocalVariable *Variable = DI.getVariable();
     DIExpression *Expression = DI.getExpression();
     dropDanglingDebugInfo(Variable, Expression);
     assert(Variable && "Missing variable");
     LLVM_DEBUG(dbgs() << "SelectionDAG visiting debug intrinsic: " << DI
                       << "\n");
     // Check if address has undef value.
     const Value *Address = DI.getVariableLocationOp(0);
     if (!Address || isa<UndefValue>(Address) ||
         (Address->use_empty() && !isa<Argument>(Address))) {
       LLVM_DEBUG(dbgs() << "Dropping debug info for " << DI
                         << " (bad/undef/unused-arg address)\n");
       return;
     }
 
     bool isParameter = Variable->isParameter() || isa<Argument>(Address);
 
-    // Check if this variable can be described by a frame index, typically
-    // either as a static alloca or a byval parameter.
-    int FI = std::numeric_limits<int>::max();
-    if (const auto *AI =
-            dyn_cast<AllocaInst>(Address->stripInBoundsConstantOffsets())) {
-      if (AI->isStaticAlloca()) {
-        auto I = FuncInfo.StaticAllocaMap.find(AI);
-        if (I != FuncInfo.StaticAllocaMap.end())
-          FI = I->second;
-      }
-    } else if (const auto *Arg = dyn_cast<Argument>(
-                   Address->stripInBoundsConstantOffsets())) {
-      FI = FuncInfo.getArgumentFrameIndex(Arg);
-    }
-
-    // llvm.dbg.declare is handled as a frame index in the MachineFunction
-    // variable table.
-    if (FI != std::numeric_limits<int>::max()) {
-      LLVM_DEBUG(dbgs() << "Skipping " << DI
-                        << " (variable info stashed in MF side table)\n");
-      return;
-    }
-
     SDValue &N = NodeMap[Address];
     if (!N.getNode() && isa<Argument>(Address))
       // Check unused arguments map.
       N = UnusedArgNodeMap[Address];
     SDDbgValue *SDV;
     if (N.getNode()) {
       if (const BitCastInst *BCI = dyn_cast<BitCastInst>(Address))
         Address = BCI->getOperand(0);

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

static bool isFoldedOrDeadInstruction(const Instruction *I,
                                       const FunctionLoweringInfo &FuncInfo) {
   return !I->mayWriteToMemory() && // Side-effecting instructions aren't folded.
          !I->isTerminator() &&     // Terminators aren't folded.
          !isa<DbgInfoIntrinsic>(I) && // Debug instructions aren't folded.
          !I->isEHPad() &&             // EH pad instructions aren't folded.
          !FuncInfo.isExportedInst(I); // Exported instrs must be computed.
 }
 
-static void processDbgDeclare(FunctionLoweringInfo &FuncInfo,
+static bool processDbgDeclare(FunctionLoweringInfo &FuncInfo,
                               const Value *Address, DIExpression *Expr,
                               DILocalVariable *Var, DebugLoc DbgLoc) {
   if (!Address) {
     LLVM_DEBUG(dbgs() << "processDbgDeclares skipping " << *Var
                       << " (bad address)\n");
-    return;
+    return false;
   }
   MachineFunction *MF = FuncInfo.MF;
   const DataLayout &DL = MF->getDataLayout();
 
   assert(Var && "Missing variable");
   assert(DbgLoc && "Missing location");
 
   // Look through casts and constant offset GEPs. These mostly come from
   // inalloca.
   APInt Offset(DL.getTypeSizeInBits(Address->getType()), 0);
   Address = Address->stripAndAccumulateInBoundsConstantOffsets(DL, Offset);
 
   // Check if the variable is a static alloca or a byval or inalloca
   // argument passed in memory. If it is not, then we will ignore this
   // intrinsic and handle this during isel like dbg.value.
   int FI = std::numeric_limits<int>::max();
   if (const auto *AI = dyn_cast<AllocaInst>(Address)) {
     auto SI = FuncInfo.StaticAllocaMap.find(AI);
     if (SI != FuncInfo.StaticAllocaMap.end())
       FI = SI->second;
   } else if (const auto *Arg = dyn_cast<Argument>(Address))
     FI = FuncInfo.getArgumentFrameIndex(Arg);
 
   if (FI == std::numeric_limits<int>::max())
-    return;
+    return false;
 
   if (Offset.getBoolValue())
     Expr = DIExpression::prepend(Expr, DIExpression::ApplyOffset,
                                  Offset.getZExtValue());
 
   LLVM_DEBUG(dbgs() << "processDbgDeclare: setVariableDbgInfo Var=" << *Var
                     << ", Expr=" << *Expr << ",  FI=" << FI
                     << ", DbgLoc=" << DbgLoc << "\n");
   MF->setVariableDbgInfo(Var, Expr, FI, DbgLoc);
+  return true;
 }
 
 /// Collect llvm.dbg.declare information. This is done after argument lowering
 /// in case the declarations refer to arguments.
 static void processDbgDeclares(FunctionLoweringInfo &FuncInfo) {
-  for (const BasicBlock &BB : *FuncInfo.Fn) {
-    for (const Instruction &I : BB) {
-      if (const DbgDeclareInst *DI = dyn_cast<DbgDeclareInst>(&I)) {
-        processDbgDeclare(FuncInfo, DI->getAddress(), DI->getExpression(),
-                          DI->getVariable(), DI->getDebugLoc());
-      }
-    }
+  for (const auto &I : instructions(*FuncInfo.Fn)) {
+    const auto *DI = dyn_cast<DbgDeclareInst>(&I);
+    if (DI && processDbgDeclare(FuncInfo, DI->getAddress(), DI->getExpression(),
+                                DI->getVariable(), DI->getDebugLoc()))
+      FuncInfo.PreprocessedDbgDeclares.insert(DI);
   }
 }
 
 /// Collect single location variable information generated with assignment
 /// tracking. This is done after argument lowering in case the declarations
 /// refer to arguments.
 static void processSingleLocVars(FunctionLoweringInfo &FuncInfo,
                                  FunctionVarLocs const *FnVarLocs) {