LGTM
On 16 May 2016 at 14:03, Steven Wu <steve...@apple.com> wrote: > >> On May 16, 2016, at 6:52 AM, Rafael Espíndola <rafael.espind...@gmail.com> >> wrote: >> >> + auto Used = collectUsedGlobalVariables(*M, UsedGlobals, true); >> >> Please use an explicit type instead of auto. > > Sure. > >> >> You deleted the assert > > Are you referring to the assertion that llvm.embedded.module has no uses. I > added it back to check it has exactly one use. > > Attach the final patch with testcase. > > Steven > > > > >> >> I think this is fine otherwise. >> >> Cheers, >> Rafael >> >> >> >> >> >> On 13 May 2016 at 20:00, Steven Wu <steve...@apple.com> wrote: >>> >>>> On May 13, 2016, at 3:56 PM, Duncan P. N. Exon Smith >>>> <dexonsm...@apple.com> wrote: >>>> >>>> Is this something that you need the linker to treat as "used", or just >>>> something you don't want the compiler to drop? If the latter, >>>> @llvm.compiler.used seems more appropriate. >>> >>> Bitcode is actually not really used by the compiler during compilation and >>> shouldn't really used by any part of the code. llvm.compiler.used then. >>> >>> New patch. >>> >>> >>> >>> >>> Steven >>> >>>> >>>>> On 2016-May-13, at 14:01, Steven Wu <steve...@apple.com> wrote: >>>>> >>>>> Attach a patch using private linkage type and adding to llvm.used. I have >>>>> to recreate llvm.used when embedding bitcode. I don't really like it but >>>>> I don't have better solutions. >>>>> Few other options: >>>>> 1. Not allowing re-embedded bitcode will simplify the code a bit but not >>>>> a lot. >>>>> 2. Create a new "llvm.*.used" variable in llvm.metadata section. >>>>> 3. Teach optimizer do not optimize away llvm.* variables. >>>>> >>>>> <0001-Fix-embed-bitcode-linkage-type.patch> >>>>> >>>>> Steven >>>>> >>>>>> On May 13, 2016, at 10:10 AM, Rafael Espíndola >>>>>> <rafael.espind...@gmail.com> wrote: >>>>>> >>>>>> On 13 May 2016 at 13:02, Steven Wu <steve...@apple.com> wrote: >>>>>>> Hi Rafael >>>>>>> >>>>>>> Thanks for notice this! That would definitely cause duplicated symbol >>>>>>> error and I should definitely change that. >>>>>>> Here is some background: >>>>>>> ld64 in Xcode 7+ knows how to handle the embedded bitcode correctly but >>>>>>> not the ones in earlier Xcode. The old ld64 will simply concatenate the >>>>>>> bitcode files which is not the right thing to do. So there is a symbol >>>>>>> generated at the place to prevent user to link the bitcode object file >>>>>>> with old ld64 because older ld64 will fail and report duplicated >>>>>>> symbols. >>>>>>> I have a radar tracking to change the linkage type when upstream but I >>>>>>> dropped the ball on that one. The correct thing to do is to make it >>>>>>> internal and add to llvm.used. I will come up with a patch. >>>>>> >>>>>> Thank you so much! >>>>>> >>>>>> Cheers, >>>>>> Rafael >>>>> >>>> >>> >>> > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
