[PATCH] D81894: [llvm-readobj][COFF] add .llvm.call-graph-profile section dump

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/COFF/call-graph-profile-err.s:6
+
+    .section .test
+a:
----------------
jhenderson wrote:
> You indent this line, but not the other `.section` directive. You should do both.
Sorry: actually on second-thoughts, I think it looks better to NOT indent the .section directives. You might want to indent the contents though (i.e. the .long/.byte directives), but entirely up to you. Same applies for the other test.


================
Comment at: llvm/test/tools/llvm-readobj/COFF/call-graph-profile-err.s:5-9
+# In order to use --cg-profile option, the section ".llvm.call-graph-profile" 
+# should have two 4-bytes data representing the indexes of two symbols and
+# one 8-bytes data representing the weight from first symbol to second 
+# symbol.
+# ".llvm.call-graph-profile" section in the test case has 9 bytes data, so it's
----------------
jhenderson wrote:
> Most new tools tests at least in this area use '##' for comments, as it helps the comment stand out from the lit/FileCheck directives. Please update to match, both here and elsewhere in your tests.
Some minor wording suggestions:

"two 4-bytes data" -> "two 4-byte fields"
"one 8-bytes data" -> "one 8-byte field"
".llvm.call-graph-profile" section in the test case -> The section in this test case
"has 9 bytes data" -> "has 9 bytes of data"


================
Comment at: llvm/test/tools/llvm-readobj/COFF/call-graph-profile-err.s:5-10
+# In order to use --cg-profile option, the section ".llvm.call-graph-profile" 
+# should have two 4-bytes data representing the indexes of two symbols and
+# one 8-bytes data representing the weight from first symbol to second 
+# symbol.
+# ".llvm.call-graph-profile" section in the test case has 9 bytes data, so it's
+# malformed.
----------------
Most new tools tests at least in this area use '##' for comments, as it helps the comment stand out from the lit/FileCheck directives. Please update to match, both here and elsewhere in your tests.


================
Comment at: llvm/test/tools/llvm-readobj/COFF/call-graph-profile-err.s:22-23
+    .section ".llvm.call-graph-profile"
+.long 10    # symbol index of a
+.long 11    # symbol index of b
+.byte 32    # weight from a to b. It is an error, since it should be have length of 8 bytes.
----------------
symbol index of a -> Symbol index of a.

(missing full stop + capital letter)
(same for b)


================
Comment at: llvm/test/tools/llvm-readobj/COFF/call-graph-profile-err.s:24
+.long 11    # symbol index of b
+.byte 32    # weight from a to b. It is an error, since it should be have length of 8 bytes.
----------------
weight -> Weight
should be have length of -> should have a length of


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81894/new/

https://reviews.llvm.org/D81894