[PATCH] Move logic for calculating DBG_VALUE history map into separate file/class.

David Blaikie dblaikie at gmail.com
Wed Apr 30 14:15:22 PDT 2014


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1444
@@ -1459,9 +1443,3 @@
         const MDNode *Var = MI->getDebugVariable();
-
-        // Variable is in a register, we need to check for clobbers.
-        if (isDbgValueInDefinedReg(MI))
-          LiveUserVar[MI->getOperand(0).getReg()] = Var;
-
-        // Check the history of this variable.
-        SmallVectorImpl<const MachineInstr *> &History = DbgValues[Var];
-        if (History.empty()) {
+        if (DbgValues.count(Var) == 0) {
+          // Create empty history for Var.
----------------
Alexey Samsonov wrote:
> David Blaikie wrote:
> > This seems like a slightly strange block.
> > 
> > Could you explain what the distinction between a variable not being in the DbgValues map, and it being in the map, but empty?
> > 
> > Also - "DbgValues[Var].clear()" is a little unclear - since the 'clear()' is actually not used - the condition above shows that the map didn't have the key until this point, so when it's created by op[] it should be empty anyway.
> > 
> > If this is the right semantic (see first question in this comment) I'd probably write it as:
> > 
> > auto IterBool = DbgValues.insert(std::make_pair(Var, SmallVector<...>()));
> > if (IterPair.second)
> >   UserVariables.push_back(Var);
> > 
> > (I wouldn't even mind rolling it into the if:
> > 
> > if (DbgValues.insert(std::make_pair(Var, SmallVector<...>())).second)
> >   UserVariables.push_back(Var);
> > 
> > )
> > 
> > Could add a comment // ensure the Var is in the DbgValues map (using some more domain-specific wording, I haven't stared at this code long enough to understand why it needs to be in the map or what that means, etc)
> > 
> > 
> Done.
Sorry, perhaps it got lost in the length of my original comment, but I still don't fully understand the need for this code. Could you explain it to me?

To quote my original comment:

"Could you explain what the distinction between a variable not being in the DbgValues map, and it being in the map, but empty?"

http://reviews.llvm.org/D3573






More information about the llvm-commits mailing list