[PATCH] D73167: Don't separate imp/expl def handling for call site params

[David Stenberg via Phabricator via llvm-commits]

dstenb created this revision.
dstenb added reviewers: djtodoro, NikolaPrica, aprantl, vsk.
dstenb added a project: debug-info.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Since D70431 <https://reviews.llvm.org/D70431> the describeLoadedValue() hook takes a parameter register,
meaning that it can now be asked to describe any register. This means
that we can drop the difference between explicit and implicit defines
that we previously had in collectCallSiteParameters().

I have not found any case for any upstream targets where a parameter
register is only implicitly defined, and does not overlap with any
explicit defines. I don't know if such a case would even make sense. So
as far as I have tested, this patch should be a non-functional change.
However, this reduces the complexity of the code a bit, and it will
simplify the implementation of an upcoming patch which solves PR44118.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73167

Files:
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp


Index: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
===================================================================
--- llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -581,35 +581,27 @@
   // the describeLoadedValue()). For those remaining arguments in the working
   // list, for which we do not describe a loaded value by
   // the describeLoadedValue(), we try to generate an entry value expression
-  // for their call site value desctipion, if the call is within the entry MBB.
+  // for their call site value description, if the call is within the entry MBB.
   // The RegsForEntryValues maps a forwarding register into the register holding
   // the entry value.
   // TODO: Handle situations when call site parameter value can be described
-  // as the entry value within basic blocks other then the first one.
+  // as the entry value within basic blocks other than the first one.
   bool ShouldTryEmitEntryVals = MBB->getIterator() == MF->begin();
   DenseMap<unsigned, unsigned> RegsForEntryValues;
 
   // If the MI is an instruction defining one or more parameters' forwarding
-  // registers, add those defines. We can currently only describe forwarded
-  // registers that are explicitly defined, but keep track of implicit defines
-  // also to remove those registers from the work list.
+  // registers, add those defines.
   auto getForwardingRegsDefinedByMI = [&](const MachineInstr &MI,
-                                          SmallVectorImpl<unsigned> &Explicit,
-                                          SmallVectorImpl<unsigned> &Implicit) {
+                                          SmallSetVector<unsigned, 4> &Defs) {
     if (MI.isDebugInstr())
       return;
 
     for (const MachineOperand &MO : MI.operands()) {
       if (MO.isReg() && MO.isDef() &&
           Register::isPhysicalRegister(MO.getReg())) {
-        for (auto FwdReg : ForwardedRegWorklist) {
-          if (TRI->regsOverlap(FwdReg, MO.getReg())) {
-            if (MO.isImplicit())
-              Implicit.push_back(FwdReg);
-            else
-              Explicit.push_back(FwdReg);
-          }
-        }
+        for (auto FwdReg : ForwardedRegWorklist)
+          if (TRI->regsOverlap(FwdReg, MO.getReg()))
+            Defs.insert(FwdReg);
       }
     }
   };
@@ -641,18 +633,17 @@
     if (ForwardedRegWorklist.empty())
       return;
 
-    SmallVector<unsigned, 4> ExplicitFwdRegDefs;
-    SmallVector<unsigned, 4> ImplicitFwdRegDefs;
-    getForwardingRegsDefinedByMI(*I, ExplicitFwdRegDefs, ImplicitFwdRegDefs);
-    if (ExplicitFwdRegDefs.empty() && ImplicitFwdRegDefs.empty())
+    SmallSetVector<unsigned, 4> FwdRegDefs;
+    getForwardingRegsDefinedByMI(*I, FwdRegDefs);
+    if (FwdRegDefs.empty())
       continue;
 
     // If the MI clobbers more then one forwarding register we must remove
     // all of them from the working list.
-    for (auto Reg : concat<unsigned>(ExplicitFwdRegDefs, ImplicitFwdRegDefs))
+    for (auto Reg : FwdRegDefs)
       ForwardedRegWorklist.erase(Reg);
 
-    for (auto ParamFwdReg : ExplicitFwdRegDefs) {
+    for (auto ParamFwdReg : FwdRegDefs) {
       if (auto ParamValue = TII->describeLoadedValue(*I, ParamFwdReg)) {
         if (ParamValue->first.isImm()) {
           int64_t Val = ParamValue->first.getImm();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73167.239526.patch
Type: text/x-patch
Size: 3325 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200122/1da322ae/attachment.bin>