[PATCH] D88717: [obj2yaml] [yaml2obj] Add yaml support for SHT_LLVM_BB_ADDR_MAP section.

[Rahman Lavaee via Phabricator via llvm-commits]

rahmanl added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml:40
+# CHECK:        Size:
+# CHECK-SAME:   0
+
----------------
jhenderson wrote:
> Watch out - this will also match `Size: 10`. You need extra regex patterns to make sure there are no preceding characters, e.g:
> ```
> # CHECK:      Size:{{ }}
> # CHECK-SAME: {{^}}0
> ```
> (in this context the `^` character represents the end of the previous match).
Thanks for the catch.
Applied your suggestion with some minor changes.


================
Comment at: llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml:114
+    Type:    SHT_LLVM_BB_ADDR_MAP
+    Entries:
+      - BBEntries:
----------------
grimar wrote:
> grimar wrote:
> > I think you should be able to write just `Entries: []`. No need to provide more input data than needed.
> > The same for Context: a single byte should be enough for this test.
> Or even: `Content: ""`
Used `-DCONTENT="00"`


================
Comment at: llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml:139
+            Metadata:         0x00000003
+    Size:    12
----------------
grimar wrote:
> I'd merge last two YAMLs into one with the use of macros. I think something like the
> following should work:
> 
> 
> ```
> --- !ELF
> FileHeader:
>   Class: ELFCLASS64
>   Data:  ELFDATA2LSB
>   Type:  ET_EXEC
> Sections:
> ##  Specify Content and Entries
>   - Name:    '.llvm_bb_addr_map'
>     Type:    SHT_LLVM_BB_ADDR_MAP
>     Entries: []
>     Content: [[CONTENT=<none>]]
>     Size:    [[SIZE=<none>]]
> ```
> 
> And then you can:
> 
> ```
> not yaml2obj --docnum=2 -DCONTENT="00"...
> not yaml2obj --docnum=2 -DSIZE=0...
> ```
> 
Looks neat. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88717