Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-16 Thread Steven Wu via cfe-commits
Thanks! Committed in r269679 Steven > On May 16, 2016, at 11:54 AM, Rafael Espíndola > wrote: > > LGTM > > On 16 May 2016 at 14:03, Steven Wu wrote: >> >>> On May 16, 2016, at 6:52 AM, Rafael Espíndola >>> wrote: >>> >>> + auto Used = collectUsedGlobalVariables(*M, UsedGlobals, true); >

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-16 Thread Rafael Espíndola via cfe-commits
LGTM On 16 May 2016 at 14:03, Steven Wu wrote: > >> On May 16, 2016, at 6:52 AM, Rafael Espíndola >> wrote: >> >> + auto Used = collectUsedGlobalVariables(*M, UsedGlobals, true); >> >> Please use an explicit type instead of auto. > > Sure. > >> >> You deleted the assert > > Are you referring t

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-16 Thread Steven Wu via cfe-commits
> On May 16, 2016, at 6:52 AM, Rafael Espíndola > 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 adde

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-16 Thread Rafael Espíndola via cfe-commits
+ auto Used = collectUsedGlobalVariables(*M, UsedGlobals, true); Please use an explicit type instead of auto. You deleted the assert I think this is fine otherwise. Cheers, Rafael On 13 May 2016 at 20:00, Steven Wu wrote: > >> On May 13, 2016, at 3:56 PM, Duncan P. N. Exon Smith >> wro

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-13 Thread Steven Wu via cfe-commits
> On May 13, 2016, at 3:56 PM, Duncan P. N. Exon Smith > 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

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-13 Thread Duncan P. N. Exon Smith via cfe-commits
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. > On 2016-May-13, at 14:01, Steven Wu wrote: > > Attach a patch using private linkage type and adding to llvm.used.

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-13 Thread Rafael Espíndola via cfe-commits
You can probably use collectUsedGlobalVariables. You can probably also delete the FIXME about llvm.used and appending linkage. I think the way to fix this is to make "can be dropped" an independent property from the linkage and then we don't need llvm.used at all. Cheers, Rafael On 13 May 2016

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-13 Thread Steven Wu via cfe-commits
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 "llv

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-13 Thread Rafael Espíndola via cfe-commits
On 13 May 2016 at 13:02, Steven Wu 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 earl

Re: [patch] Don't use appending linkage for embeded bitcode

2016-05-13 Thread Steven Wu via cfe-commits
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 bit

[patch] Don't use appending linkage for embeded bitcode

2016-05-13 Thread Rafael Espíndola via cfe-commits
Hi Steven, I think there was a mistake when picking this linkage. The appending linkage is really just for things that llvm itself special cases. By an historical artifact it was codegened just like external. The attached patch changes it to external linkage. I tested that the produced .o file is