[PATCH] D130875: [Metadata] Introduce MD_pcsections

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: llvm/lib/IR/MDBuilder.cpp:166
+
+  for (size_t i = 0; i < Sections.size(); i++) {
+    const StringRef &S = Sections[i];
----------------
vitalybuka wrote:
> melver wrote:
> > vitalybuka wrote:
> > > is implicit mapping from Sections[i] to AuxData[i] in interface is neccecary?
> > > can you do SomeMap<StringRef, <ArrayRef<Constant *>> ?
> > > 
> > > That would be cleaner
> > Map doesn't have stable iteration order. So I'm trying ArrayRef<std::pair<StringRef, SmallVector<Constant*>>>. The problem is that the inner array can't be an ArrayRef, otherwise users of the function have to either convert to something that can convert to ArrayRef<std::pair<StringRef, ArrayRef>> ensuring that whatever inner ref they pass lives long enough.
> > 
> > The reason of the earlier interface was to avoid forcing SmallVector or any particular vector onto callers by sticking to ArrayRef. But by wrapping it into a std::pair, the ArrayRef conversion can't be done implicitly anymore, so usability suffers.
> > 
> > This means that in SanitizerBinaryMetadadata, I'm now using a SmallVector<StringRef, SmallVector<Constant*>>> (instead of just a SmallVector<StringRef>)  where the inner SmallVector will always be 0-sized, which wastes space but didn't before. Perhaps this was premature optimization.
> >> Map doesn't have stable iteration order.
> Do you need a particular section order?
> 
> pair<stringref, SmallVector> is not nice, but better than obvious mapping.
> 
> 
> maybe real builder?
> ```
> PCSectionsBuilder B =  MDB.getPCSectionsBuilder();
> F.setMetadata(
>    B.addSection(S, {c1})
>       .addSection(S, {}).
>       .addSection(S, {c1, c2})
>       .buid());
> ```
> Map doesn't have stable iteration order.
Do you need a particular section order?

pair<stringref, SmallVector> is not nice, but better than non-obvious mapping.

maybe real builder?
```
PCSectionsBuilder B =  MDB.getPCSectionsBuilder();
F.setMetadata(
   B.addSection(S, {c1})
      .addSection(S, {}).
      .addSection(S, {c1, c2})
      .build());
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130875