[PATCH] D85740: Universal MachO: support LLVM IR objects

Wed Aug 12 01:46:35 PDT 2020

aguinet added a comment.

Hello @alexshap!

Thanks for the comments. Answers below:

In D85740#2210765 <https://reviews.llvm.org/D85740#2210765>, @alexshap wrote:

> 1. This code has been recently refactored (see https://github.com/llvm/llvm-project/commit/df69492cdfa82ff6453c887cd45b3a5903b79afb ) so you'd need to rebase your diff.

I pushed a new patch rebased on master, sorry about this conflict (did this patch before going to holidays, should have rebased before submitting when coming back :)).

> 1. Bitcode files can also come from archives (static libraries)  (e.g. an archive can contain bitcode files exclusively) - so when we create a Slice from an archive we also need to handle that.

That's a good catch. I will take a closer look and adapt the patch accordingly.

> 1. We need tests.

About that part, like I said in the description, the way I see it is to enhance the ObjectYAML library so that we can generate LLVM bitcode file format (as described in https://llvm.org/docs/BitCodeFormat.html), so that yaml2obj can generate appropriate LLVM IR test objects. That would be used for instance to test the -create flag. Do you see another way to do this?

> Somewhat related to this area - If I'm not mistaken there is a small issue in SymbolicFile::createSymbolicFile - when LLVMContext is null the function appears to return a misleading error - I was planning to fix that soon.

Indeed. If LLVMCOntext is null, it says that it is in an invalid format. Not ideal what should be the error, as this is an error which is forwarded to the end user (and I'm not sure the user would understand "you need to pass an LLVMContext object to SymbolicFile::createSymbolicFile to parse IR objects" :))

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85740