[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