[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