[PATCH] D38550: [XRay][tools] Support arg1 logging entries in the basic logging mode

Martin Pelikán via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 4 21:47:28 PDT 2017


pelikan added inline comments.


================
Comment at: lib/XRay/Trace.cpp:260-263
+  // We can encounter a CustomEventMarker anywhere in the log, so we can
+  // handle it regardless of the expectation. However, we do set the
+  // expectation to read a set number of fixed bytes, as described in the
+  // metadata.
----------------
These wraps are not necessary, it fits 80 characters well.


================
Comment at: lib/XRay/Trace.cpp:295-296
 /// Record, which the caller should pass in because they have already read it
-/// to determine that this is a metadata record as opposed to a function record.
+/// to determine that this is a metadata record as opposed to a function
+/// record.
 Error processFDRMetadataRecord(FDRState &State, uint8_t RecordFirstByte,
----------------
These wraps are not necessary, it fits 80 characters well.


================
Comment at: lib/XRay/Trace.cpp:389-390
     default:
-      // Cast to an unsigned integer to not interpret the record type as a char.
+      // Cast to an unsigned integer to not interpret the record type as a
+      // char.
       return make_error<StringError>(
----------------
These wraps are not necessary, it fits 80 characters well.


================
Comment at: lib/XRay/Trace.cpp:423-433
+/// The following is an attempt to document the grammar of the format, which
+/// is parsed by this function for little-endian machines. Since the format
+/// makes use of BitFields, when we support big-endian architectures, we will
+/// need to adjust not only the endianness parameter to llvm's
+/// RecordExtractor, but also the bit twiddling logic, which is consistent
+/// with the little-endian convention that BitFields within a struct will
+/// first be packed into the least significant bits the address they belong
----------------
These wraps are not necessary, it fits 80 characters well.


https://reviews.llvm.org/D38550





More information about the llvm-commits mailing list