[DeadArgumentElim] Set pointer to DISubprogram before calling RAUW. NFC

[djtodoro]

Summary

It is better to update pointer of the DISuprogram before we call RAUW for still live arguments of the function, because with D42541 in RAUW we compare DISubprograms rather than functions itself.

[aprantl]

The change LGTM, but now I wonder why this didn't break any tests. Are we missing coverage here?

[djtodoro]

@aprantl All test cases pass. That is why RAUW has redundant check (getLocalFunctionMetadata for From and To). Maybe in some cases LLVM PASS just did not get the place where actually it updates the pointer to the DISubprogram (or it was pointer to the function before D42541) and if it is null at that particular point that does not mean that it clobbers dbg info (because it would be != than other metadata at the time).

[aprantl]

ok

[mattd]

@djtodoro thanks, this change looks nice, it makes the code easier to read too!
Just a suggestion, could we just set the Subprogram earlier where we set all of the
more basic properties of the function, when NF is created around line 819, or is
that too soon?

[djtodoro]

@mattd Thanks! I don't see any obstacle to do that, it sounds good to me to set it even earlier.
@aprantl what do you think?

[aprantl]

Works for me.

[mattd]

Thanks @djtodoro! LGTM

lib/Transforms/IPO/DeadArgumentElimination.cpp

while (!F->use_empty()) {
     Call->eraseFromParent();
   }
 
   // Since we have now created the new function, splice the body of the old
   // function right into the new function, leaving the old rotting hulk of the
   // function empty.
   NF->getBasicBlockList().splice(NF->begin(), F->getBasicBlockList());
 
+  // Patch the pointer to LLVM function in debug info descriptor.
+  NF->setSubprogram(F->getSubprogram());
+
   // Loop over the argument list, transferring uses of the old arguments over to
   // the new arguments, also transferring over the names as well.
   i = 0;
   for (Function::arg_iterator I = F->arg_begin(), E = F->arg_end(),
        I2 = NF->arg_begin(); I != E; ++I, ++i)
     if (ArgAlive[i]) {
       // If this is a live argument, move the name and users over to the new
       // version.
for (BasicBlock &BB : *NF)
             }
         }
         // Replace the return instruction with one returning the new return
         // value (possibly 0 if we became void).
         ReturnInst::Create(F->getContext(), RetVal, RI);
         BB.getInstList().erase(RI);
       }
 
-  // Patch the pointer to LLVM function in debug info descriptor.
-  NF->setSubprogram(F->getSubprogram());
-
   // Now that the old function is dead, delete it.
   F->eraseFromParent();
 
   return true;
 }
 
 PreservedAnalyses DeadArgumentEliminationPass::run(Module &M,
                                                    ModuleAnalysisManager &) {