[PATCH] Add parser for the stackmap section format
Sanjoy Das
sanjoy at playingwithpointers.com
Wed Jun 10 18:23:01 PDT 2015
This change mostly has stylistic issues -- I've commented on some; and whitespace needs to be fixed (I'd just run the change through `clang-format`).
As far as testing is concerned, does it make sense to run the parser over a binary blob in a test in `unittests/`?
================
Comment at: lib/CodeGen/StackMaps.cpp:39
@@ +38,3 @@
+template <typename Primitive>
+static Primitive parse_primitive(uint8_t* data, unsigned& offset,
+ const unsigned len) {
----------------
Should be `ParsePrimitive`.
================
Comment at: lib/CodeGen/StackMaps.cpp:43
@@ +42,3 @@
+ assert(offset < len);
+ Primitive rval = *(Primitive*)(data + offset);
+ offset += sizeof(rval);
----------------
So the machine generating the stackmap section and the machine parsing them should have the same endianness -- should this be documented explicitly?
================
Comment at: lib/CodeGen/StackMaps.cpp:49
@@ +48,3 @@
+
+void LocationRecord::parse(uint8_t* data, unsigned& offset,
+ const unsigned len) {
----------------
LLVM style is `uint8_t *Data` etc.
================
Comment at: lib/CodeGen/StackMaps.cpp:114
@@ +113,3 @@
+ printf("Functions (%d) [\n", static_cast<int>(FnSizeRecords.size()));
+ for (uint16_t j = 0; j < FnSizeRecords.size(); j++) {
+ printf(" addr=%u, size=%u", static_cast<unsigned>(FnSizeRecords[j].FunctionAddr),
----------------
foreach?
================
Comment at: lib/CodeGen/StackMaps.cpp:124
@@ +123,3 @@
+ printf("Constants (%d) [\n", static_cast<int>(Constants.size()));
+ for (uint16_t j = 0; j < Constants.size(); j++) {
+ printf(" value=%u", static_cast<unsigned>(Constants[j]));
----------------
foreach?
http://reviews.llvm.org/D10377
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list