[PATCH] Move logic for calculating DBG_VALUE history map into separate file/class.
Alexey Samsonov
samsonov at google.com
Wed Apr 30 11:31:58 PDT 2014
Sorry for the lack of context.
I'm preparing the ground for reviving http://reviews.llvm.org/D2658, which attempts to heavily refactor history calculation. It's probably a good idea to factor this logic out of beginFunction() anyway, as it doesn't require any knowledge of DwarfDebug contents.
================
Comment at: lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.h:25
@@ +24,3 @@
+ const MachineFunction *MF;
+ const TargetRegisterInfo *TRI;
+
----------------
David Blaikie wrote:
> 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).
For now, when the class doesn't yet have a state, I can turn certainly turn it into a function for simplicity.
================
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
----------------
David Blaikie wrote:
> 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.
I'd prefer to leave braces here - else-block occupies multiple lines, has a comment etc.
http://reviews.llvm.org/D3573
More information about the llvm-commits
mailing list