[NFC][SelectionDAG][DebugInfo] Remove duplicate parameter from handleDebugValue

[Orlando]

Summary

[NFC][SelectionDAG][DebugInfo] Remove duplicate parameter from handleDebugValue

handleDebugValue has two DebugLoc parameters that appear to always take the
same value. Remove one of the duplicate parameters.

Now an attempt to disentangle the code and explain why I think they are always
the same value:

The call to handleDebugValue in visitIntrinsicCall uses dl and
DI.getDebugLoc() as the two DebugLoc arguments, where DI is the
current instruction being visited casted to a DbgValueInst.

At the start of visitIntrinsicCall we can see

DebugLoc dl = getCurDebugLoc();

Here's the definition for getCurDebugLoc:

DebugLoc getCurDebugLoc() const {
  return CurInst ? CurInst->getDebugLoc() : DebugLoc();
}

Noting that CurInst is set to the instruction-being-visited in
SelectionDAGBuilder::visit(const Instruction &I) before calling visit
which results in the call to visitIntrinsicCall. This shows that dl
and DI.getDebugLoc() are the same.

The other two calls to handleDebugValue occur inside
SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI). The
two DebugLocs passed to handleDebugValue from here are:

DebugLoc DL = DDI.getdl();
DebugLoc InstDL = DDI.getDI()->getDebugLoc();

The DanglingDebugInfo parameter DDI is created by the call from
visitIntrinsicCall to addDanglingDebugInfo:

addDanglingDebugInfo(&DI, dl, SDNodeOrder);

So following the logic for the arguments to the call to handleDebugValue in
visitIntrinsicCall, DDI.getdl() and DDI.getDI()->getDebugLoc() used in
salvageUnresolvedDbgValue are also the same value.

To help test this I added an assert to check the arguments were equal and built
the CTMark test suite - the assert wasn't tripped. That's not conclusive, but
adds confidence.

---

I'll put up another NFC patch that removes unneeded DebugLoc field from
DanglingDebugInfo shortly.

As well as being a nice tidy-up, these changes is used by an upcoming assignment tracking patch.

[StephenTozer]

LGTM, doing a bit of archaeology doesn't reveal any reason why we would expect dl != DI.getDebugLoc(), it seems as though the same thing was just written in two different ways and then embedded as parameters when the code was extracted to a new function. That being said, just pinging @jmorse to double check that there is no intentional difference between the two recorded locations.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

public:
   /// For the given dangling debuginfo record, perform last-ditch efforts to
   /// resolve the debuginfo to something that is represented in this DAG. If
   /// this cannot be done, produce an Undef debug value record.
   void salvageUnresolvedDbgValue(DanglingDebugInfo &DDI);
 
   /// For a given list of Values, attempt to create and record a SDDbgValue in
   /// the SelectionDAG.
   bool handleDebugValue(ArrayRef<const Value *> Values, DILocalVariable *Var,
-                        DIExpression *Expr, DebugLoc CurDL, DebugLoc InstDL,
-                        unsigned Order, bool IsVariadic);
+                        DIExpression *Expr, DebugLoc DbgLoc, unsigned Order,
+                        bool IsVariadic);
 
   /// Evict any dangling debug information, attempting to salvage it first.
   void resolveOrClearDbgInfo();
 
   SDValue getValue(const Value *V);
 
   SDValue getNonRegisterValue(const Value *V);
   SDValue getValueImpl(const Value *V);

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI) {
   // invalid, so that we attempt to salvage only those values when processing
   // a DIArgList.
   assert(!DDI.getDI()->hasArgList() &&
          "Not implemented for variadic dbg_values");
   Value *V = DDI.getDI()->getValue(0);
   DILocalVariable *Var = DDI.getDI()->getVariable();
   DIExpression *Expr = DDI.getDI()->getExpression();
   DebugLoc DL = DDI.getdl();
-  DebugLoc InstDL = DDI.getDI()->getDebugLoc();
   unsigned SDOrder = DDI.getSDNodeOrder();
   // Currently we consider only dbg.value intrinsics -- we tell the salvager
   // that DW_OP_stack_value is desired.
   assert(isa<DbgValueInst>(DDI.getDI()));
   bool StackValue = true;
 
   // Can this Value can be encoded without any further work?
-  if (handleDebugValue(V, Var, Expr, DL, InstDL, SDOrder, /*IsVariadic=*/false))
+  if (handleDebugValue(V, Var, Expr, DL, SDOrder, /*IsVariadic=*/false))
     return;
 
   // Attempt to salvage back through as many instructions as possible. Bail if
   // a non-instruction is seen, such as a constant expression or global
   // variable. FIXME: Further work could recover those too.
   while (isa<Instruction>(V)) {
     Instruction &VAsInst = *cast<Instruction>(V);
     // Temporary "0", awaiting real implementation.
while (isa<Instruction>(V)) {
     if (!AdditionalValues.empty())
       break;
 
     // New value and expr now represent this debuginfo.
     Expr = DIExpression::appendOpsToArg(Expr, Ops, 0, StackValue);
 
     // Some kind of simplification occurred: check whether the operand of the
     // salvaged debug expression can be encoded in this DAG.
-    if (handleDebugValue(V, Var, Expr, DL, InstDL, SDOrder,
-                         /*IsVariadic=*/false)) {
+    if (handleDebugValue(V, Var, Expr, DL, SDOrder, /*IsVariadic=*/false)) {
       LLVM_DEBUG(dbgs() << "Salvaged debug location info for:\n  "
                         << *DDI.getDI() << "\nBy stripping back to:\n  " << *V);
       return;
     }
   }
 
   // This was the final opportunity to salvage this debug information, and it
   // couldn't be done. Place an undef DBG_VALUE at this location to terminate
   // any earlier variable location.
   auto Undef = UndefValue::get(DDI.getDI()->getValue(0)->getType());
   auto SDV = DAG.getConstantDbgValue(Var, Expr, Undef, DL, SDNodeOrder);
   DAG.AddDbgValue(SDV, false);
 
   LLVM_DEBUG(dbgs() << "Dropping debug value info for:\n  " << *DDI.getDI()
                     << "\n");
   LLVM_DEBUG(dbgs() << "  Last seen at:\n    " << *DDI.getDI()->getOperand(0)
                     << "\n");
 }
 
 bool SelectionDAGBuilder::handleDebugValue(ArrayRef<const Value *> Values,
                                            DILocalVariable *Var,
-                                           DIExpression *Expr, DebugLoc dl,
-                                           DebugLoc InstDL, unsigned Order,
-                                           bool IsVariadic) {
+                                           DIExpression *Expr, DebugLoc DbgLoc,
+                                           unsigned Order, bool IsVariadic) {
   if (Values.empty())
     return true;
   SmallVector<SDDbgOperand> LocationOps;
   SmallVector<SDNode *> Dependencies;
   for (const Value *V : Values) {
     // Constant value.
     if (isa<ConstantInt>(V) || isa<ConstantFP>(V) || isa<UndefValue>(V) ||
         isa<ConstantPointerNull>(V)) {
for (const Value *V : Values) {
     // Do not use getValue() in here; we don't want to generate code at
     // this point if it hasn't been done yet.
     SDValue N = NodeMap[V];
     if (!N.getNode() && isa<Argument>(V)) // Check unused arguments map.
       N = UnusedArgNodeMap[V];
     if (N.getNode()) {
       // Only emit func arg dbg value for non-variadic dbg.values for now.
       if (!IsVariadic &&
-          EmitFuncArgumentDbgValue(V, Var, Expr, dl,
+          EmitFuncArgumentDbgValue(V, Var, Expr, DbgLoc,
                                    FuncArgumentDbgValueKind::Value, N))
         return true;
       if (auto *FISDN = dyn_cast<FrameIndexSDNode>(N.getNode())) {
         // Construct a FrameIndexDbgValue for FrameIndexSDNodes so we can
         // describe stack slot locations.
         //
         // Consider "int x = 0; int *px = &x;". There are two kinds of
         // interesting debug values here after optimization:
for (const Value *V : Values) {
     }
 
     const TargetLowering &TLI = DAG.getTargetLoweringInfo();
     // Special rules apply for the first dbg.values of parameter variables in a
     // function. Identify them by the fact they reference Argument Values, that
     // they're parameters, and they are parameters of the current function. We
     // need to let them dangle until they get an SDNode.
     bool IsParamOfFunc =
-        isa<Argument>(V) && Var->isParameter() && !InstDL.getInlinedAt();
+        isa<Argument>(V) && Var->isParameter() && !DbgLoc.getInlinedAt();
     if (IsParamOfFunc)
       return false;
 
     // The value is not used in this block yet (or it would have an SDNode).
     // We still want the value to appear for the user if possible -- if it has
     // an associated VReg, we can refer to that instead.
     auto VMI = FuncInfo.ValueMap.find(V);
     if (VMI != FuncInfo.ValueMap.end()) {
if (VMI != FuncInfo.ValueMap.end()) {
           unsigned FragmentSize = (Offset + RegisterSize > BitsToDescribe)
                                       ? BitsToDescribe - Offset
                                       : RegisterSize;
           auto FragmentExpr = DIExpression::createFragmentExpression(
               Expr, Offset, FragmentSize);
           if (!FragmentExpr)
             continue;
           SDDbgValue *SDV = DAG.getVRegDbgValue(
-              Var, *FragmentExpr, RegAndSize.first, false, dl, SDNodeOrder);
+              Var, *FragmentExpr, RegAndSize.first, false, DbgLoc, SDNodeOrder);
           DAG.AddDbgValue(SDV, false);
           Offset += RegisterSize;
         }
         return true;
       }
       // We can use simple vreg locations for variadic dbg_values as well.
       LocationOps.emplace_back(SDDbgOperand::fromVReg(Reg));
       continue;
     }
     // We failed to create a SDDbgOperand for V.
     return false;
   }
 
   // We have created a SDDbgOperand for each Value in Values.
   // Should use Order instead of SDNodeOrder?
   assert(!LocationOps.empty());
-  SDDbgValue *SDV =
-      DAG.getDbgValueList(Var, Expr, LocationOps, Dependencies,
-                          /*IsIndirect=*/false, dl, SDNodeOrder, IsVariadic);
+  SDDbgValue *SDV = DAG.getDbgValueList(Var, Expr, LocationOps, Dependencies,
+                                        /*IsIndirect=*/false, DbgLoc,
+                                        SDNodeOrder, IsVariadic);
   DAG.AddDbgValue(SDV, /*isParameter=*/false);
   return true;
 }
 
 void SelectionDAGBuilder::resolveOrClearDbgInfo() {
   // Try to fixup any remaining dangling debug info -- and drop it if we can't.
   for (auto &Pair : DanglingDebugInfoMap)
     for (auto &DDI : Pair.second)
case Intrinsic::dbg_value: {
     SmallVector<Value *, 4> Values(DI.getValues());
     if (Values.empty())
       return;
 
     if (llvm::is_contained(Values, nullptr))
       return;
 
     bool IsVariadic = DI.hasArgList();
-    if (!handleDebugValue(Values, Variable, Expression, dl, DI.getDebugLoc(),
+    if (!handleDebugValue(Values, Variable, Expression, DI.getDebugLoc(),
                           SDNodeOrder, IsVariadic))
-      addDanglingDebugInfo(&DI, dl, SDNodeOrder);
+      addDanglingDebugInfo(&DI, DI.getDebugLoc(), SDNodeOrder);
     return;
   }
 
   case Intrinsic::eh_typeid_for: {
     // Find the type id for the given typeinfo.
     GlobalValue *GV = ExtractTypeInfo(I.getArgOperand(0));
     unsigned TypeID = DAG.getMachineFunction().getTypeIDFor(GV);
     Res = DAG.getConstant(TypeID, sdl, MVT::i32);