[PATCH] D60974: Clang IFSO driver action.

Mon Apr 29 17:17:49 PDT 2019

plotfi added a comment.

In D60974#1483265 <https://reviews.llvm.org/D60974#1483265>, @jakehehrlich wrote:

> In D60974#1483240 <https://reviews.llvm.org/D60974#1483240>, @plotfi wrote:
>
> > Me and @compnerd had discussed a more abstracted format like this but decided it was best to just use the same names that are in the ELF already.
> >  Is there any compelling reason not to?
> >  As far as I understand, by having something like "Weak: true" is already harking back to ELF so why not stick to the same names?
> >
> > I think the !tbd-elf-v1 versioning can help with any changes or alterations we want to make along the way too.
> >  We did discuss the alignment field too.
>
>
> The format will have to be ELF specific but that doesn't mean we have to use the exact names. The benefit of this format is that you can only do the intended thing with it while anything more. This is also the format that matches most closely with .tbe which is a plus for consistency of this and integration of both tools into the llvm ecosystem. It's obvious how to merge my format into the ELFStub format. Your format has extraneous details that would never matter to the creation of the ELFStub format like the name of the section a symbol was defined in. Also I think much more of the compiler has to be considered to get section names right unless you're just recomputing them and then that's redundant for no gain.

We wanted to use the same names because its just a lot easier understand what is if you've already looked at the ELF header code (ie STT_FUNC vs Function).

> When we first reviewed the .tbe format I was on the "just use ELF name" side of things for the Type and Arch fields. I don't remember why that wasn't chosen but for consistency with that choice I think we should do the same thing here. As for fields like Weak vs not Weak distilling the format down to only allow for those options is a design benefit. not Weak means (GLOBAL or GNU_UNIQUE) and Weak means WEAK. Likewise by not messing with visability (you don't mess with it either, I'm just pointing this out) we split things into (default | protected) and (internal | hidden). These grouped concepts, though exactly what we need, are not what available concepts. We also constrain useless bindings from existing like LOCAL which isn't meaningful in this context.
> 
> Every bit of my format is needed to produce the final stub in working form. All section information in your format, some bindings, and ELF file type are all not needed. There's an argument for embedding the ELF encoding format in this file but that will only be used later once linking occurs. It can also be inferred by the architecture in most cases and otherwise specified using a bfd output format name when that's what people need/want.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974