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

Alexey Samsonov samsonov at google.com
Wed Apr 30 14:37:17 PDT 2014


Thanks for the review!

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1443
@@ -1459,1 +1442,3 @@
+        // Keep track of user variables in order of appearance. Store the set
+        // of variables we've already seen as a set of keys in DbgValues.
         const MDNode *Var = MI->getDebugVariable();
----------------
David Blaikie wrote:
> Alexey Samsonov wrote:
> > See the updated // comment here. We need to store both DbgValues map (MDNode->History) and a set of MDNodes in UserVariables in the order of appearance. There's no difference in missing MDNode and MDNode with an empty history, but we use the latter for constructing UserVariables (it will be constructed later anyway).
> > 
> > We could either fix a DbgValues map to ensure the order on keys, or introduce a temporary map we would use when constructing UserVariables here, or leave the code as is - I have no strong opinion about it.
> Oh, sorry I missed the comment. Thanks for the details.
> 
> OK, that's a pity, though not your fault/relevant to this patch. (I may've perpetrated this oddity to address weird parameter reordering shenanigans - I'm not sure)
> 
> As a follow-up patch it'd be good to change DbgValues to a MapVector. Then we wouldn't have to pay the cost of lookup in collectVariableInfo where we loop over the UserVariables then look them up in DbgValues - we could just iterate directly.
> 
> And this code would become a little less weird - though it'd still need the comment about insertion order being important.
Yep, I like the idea of using MapVector here. Will do in a following changes.

http://reviews.llvm.org/D3573






More information about the llvm-commits mailing list