[PATCH] D121346: [Propeller] Encode address offsets of basic blocks relative to the end of the previous basic blocks.

[Rahman Lavaee via Phabricator via llvm-commits]

rahmanl added a comment.
Herald added a subscriber: jsji.

In D121346#3518196 <https://reviews.llvm.org/D121346#3518196>, @jhenderson wrote:

> Has the spec for this been finalised anywhere? My main conceren is the use of section names to have semantic importance. ELF generally tries to avoid this, hence the use of section types, and it would be a shame to introduce this approach when there are other options. It would be far more preferable to include the version number in the section data somewhere, a bit like how most DWARF sections are encoded. I can think of one other possible way of doing this: change the section type value for version 1 and upwards, and rename the original value to something like SHT_LLVM_BB_ADDR_MAP_V0. Add the version field as the first N bytes (2 or 4 probably) of the new section type. Parsers understanding the old data structure only won't recognise the new section type as a recognised format. This is good because it doesn't mislead people by printing incorrect offsets (in addition to not needing to rely on the section name).

Thanks for the review.  And apologies for my delayed followup.
We did consider several ideas for storing the version.

1. Store it for each function: wasteful for object-file size
2. Store it once in a different COMDAT section of the object file and have the linker merge them all:  Would not work for mixing different versions.
3. Store it outside the section as a weak symbol. (Similar to 2).
4. Store it inside the section metadata: For example, we can suffix the section name with the version name and then have the LLVM_BB_ADDR_MAP reading code read sections of different versions.

We chose idea 4 mostly for convenience reasons.

IIUC, your suggestion is to embed the version in the section data. The problem with this approach is that the linker must read and deduplicate the version data when linking the sections (unless if we store the version for each function separately). 
Also, if we compile with different compiler versions, the linker must create multiple LLVM_BB_ADDR_MAP sections if multiple versions exist.
For these reasons I am a bit hesitant to add linker dependency to the feature, even though section-name-independence is great to have. I'd be happy to change course if we can avoid involving the linker. Any thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121346