[PATCH] [FaultMaps] Add a parser for the __llvm__faultmaps section.

[Joseph Tremoulet]

LGTM, just some minor cosmetic feedback.


================
Comment at: include/llvm/CodeGen/FaultMaps.h:75-76
@@ +74,4 @@
+/// A parser for the __llvm_faultmaps section generated by the FaultMaps class
+/// above.  This parser is version locked with with the __llvm_faultmaps section
+/// generated by the version of that includes it.  No guarantees are made with
+/// respect to forward or backward compatibility.
----------------
I think you have a typo or missing word(s) here (was it supposed to read "version of LLVM that includes it"?).

================
Comment at: include/llvm/CodeGen/FaultMaps.h:117
@@ +116,3 @@
+    uint64_t getFunctionAddr() const {
+      return read<uint64_t>(P + FunctionAddrOffset, E);
+    }
----------------
I had the same reaction as Philip on seeing the uint//nn//_t types replicated in getFoo vs the FooOffset calculation.  Did you consider introducing typedefs for each field (e.g. typedef uint64_t FunctionAddrType) so that the offset calculation and read template argument could each be in terms of the typedef, and there'd be just one place mapping each field to its type?

================
Comment at: include/llvm/CodeGen/FaultMaps.h:124
@@ +123,3 @@
+
+    FunctionFaultInfoAccessor getFunctionFaultInfoAt(uint32_t idx) const {
+      assert(idx < getNumFaultingPCs() && "index out of bounds!");
----------------
Should "idx" be "Idx" instead?

================
Comment at: include/llvm/CodeGen/FaultMaps.h:132-135
@@ +131,6 @@
+    FunctionInfoAccessor getNextFunctionInfo() const {
+      size_t MySize = sizeof(uint64_t) + // FunctionAddress
+                      sizeof(uint32_t) + // NumFaultingPCs
+                      sizeof(uint32_t) + // Reserved
+                      getNumFaultingPCs() * FunctionFaultInfoSize;
+
----------------
This could be `size_t NextFunctionInfoOffset = FunctionFaultInfosOffset + getNumFaultingPCs() * FunctionFaultInfoSize;` to avoid needing to duplicate the whole field list here.

http://reviews.llvm.org/D10491

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/