plotfi added a comment. In D60974#1490358 <https://reviews.llvm.org/D60974#1490358>, @rupprecht wrote:
> I didn't follow the technical details, but I don't see anything wrong with > moving forward on this patch. I think this seems like an interesting idea > worth experimenting with. > > In D60974#1488563 <https://reviews.llvm.org/D60974#1488563>, @jakehehrlich > wrote: > > > > Jake, I am still not sure what you would prefer for moving forward. The > > > current patch can output the yaml format I originally proposed or the one > > > that looks similar to the one you proposed. What I need to know is: > > > > > > Do you want the merging if the "ifo" files to happen inside of > > > llvm-elfabi? > > > Do you care if we upstream the yaml we proposed as a first step (which > > > is really a distilled version of that yaml2obj can consume anyways. this > > > right now functions actually) ??? > > > Or, would you rather the ifo files be merged by a different separate > > > tool (something maybe called llvm-ifsogen)??? > > > > This is my proposal: > > > > 1. We should plan on adding the code ofr merging these files inside of > > `llvm/tools/llvm-elfabi` but will it be a separate tool from `llvm-elfabi`. > > You can create "separate" tools in the same directory using the symlink > > trick. See llvm-ar and friends or llvm-objcopy and llvm-strip. The tool > > name can be discussed in review. > > > The symlink trick is usually used when the new frontend tool is just a thin > layer over the underlying tool. Is that going to be the case here? > > For example, > > - `llvm-ranlib` is equivalent to `llvm-ar s` > - `llvm-readelf` is equivalent to `llvm-readobj --elf-output-style=GNU` > - `llvm-addr2line` is equivalent to `llvm-symbolizer --output-style=GNU` > - `llvm-strip` is equivalent to `llvm-objcopy --strip-all` (maybe not exactly) > > In other words, is `llvm-mergeifo` (placeholder name) going to be roughly > equivalent to `llvm-elfabi --merge-ifo` (again, placeholder flag name)? And > if so, it doesn't seem like a symlink is strictly necessary -- users can just > call `llvm-elfabi --merge-ifo`. Am I missing something? @rupprecht I was looking into this. I think it could work to just have a secondary path for loading tbe files and merging them. I modified the tbe schema to include an "Endian" field (which I would like to emit from clang). >> 2. The yaml proposed thus far is not acceptable IMO for reasons already >> outlined but this can be discussed in code review. Before adding sections a >> clear reason for needing them is required which hasn't been given yet. >> Things can evolve and change later as needed but we should start with the >> minimum and add as needed later; not start with extra and remove later. >> Support for the new format should be added into TextAPI and in the review >> for that process we should discuss the format. After we add support for this >> new format into TextAPI we can add support for emitting this format into >> clang (well actually you can write the code whenever but I'm using "after" >> in a different sense here). > > What if we named it experimental for now? i.e. `--experimental-emit-ifo` for > the clang flag name and `!experimental-ifo-elf-v1` for the yaml id? That > would allow those working on this patch to play around with more features, > but still give sufficient warning to anyone that fields they depend on may be > removed. > > In fact, if we label it experimental (or maybe even if we don't), I don't see > any reason this couldn't land now, even without a consumer of it. So what if > a tool produces a yaml file that we don't haven't finished the yaml parser > for? Does anything break? @rupprecht I like this idea a lot, and I think it's a good compromise. I think in the long run there are multiple formats that could be output from clang-emit-interface-stubs so I went ahead and added an additional -interface-stubs-version= flag (because why limit the output format to a specific version of tbe or elf?). My current diff can generate either tbe (that can be consumed by llvm-elfabi) by default or yaml that can be consumed by yaml2obj (when passing -interface-stubs-version=experimental-ifo-elf-v1). The default output pretty much produces the exact tbe that llvm-elfabi consumes so its not really a new yaml schema here (-interface-stubs-version=tapi-tbe or the default will do this), on the other hand -interface-stubs-version=experimental-ifo-elf-v1 can be used to give us freedom to target the internal yaml2obj tool if we needed to. I still need to go back and change all the references to either "ifso" or "ifo" to "interface stub" or "ifs" for consistency. I hope this is a reasonable compromise and can land with close to what I have here. @jakehehrlich @compnerd thoughts? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60974/new/ https://reviews.llvm.org/D60974 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
