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

David Blaikie dblaikie at gmail.com
Wed Apr 30 11:16:29 PDT 2014


Any particular motivation for this change? Is it a precursor to other work?

But yeah, putting this rather monolithic complexity in its own space is probably not a bad idea, I'd just like to understand the motivation/design a bit better.

================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.h:25
@@ +24,3 @@
+  const MachineFunction *MF;
+  const TargetRegisterInfo *TRI;
+
----------------
Immediate gut reaction is that this object has no state (presumably it doesn't mutate the MachineFunction or the TargetRegisterInfo - it just uses those to perform some computation) so it should be a free function.

Arguably it could still be in its own source file - it's certainly a lot of code - but I have an aversion to classes that are just containers for functions (we have namespaces for that).

================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1449
@@ -1506,8 +1448,3 @@
         }
-        History.push_back(MI);
       } else {
         // First known non-DBG_VALUE and non-frame setup location marks
----------------
Unneeded braces on single-statement block. But I know some people prefer an "all or nothing" brace approach (if the 'if' has braces, then put braces on the 'else' too) - so that's up to you.

http://reviews.llvm.org/D3573






More information about the llvm-commits mailing list