[PATCH] D41113: [LLVMgold] Don't set resolutions for undefined symbols to 'Prevailing'

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D41113#953964, @evgeny777 wrote:

> > Can you add an assert for this
>
> `check-all` passes with this, but we can explicitly mark asm undefined symbol as prevailing using llvm-lto(2) and trigger this assert. Is this OK?


Not sure what you mean about marking via llvm-lto2 - I meant that the asm_undefined2.ll test should cause this assert to fire without the fix in this patch (i.e. suppress the fix temporarily to make sure it fires there).

> 
> 
>> What happens if the asm does have the prevailing def? Isn't that possible? In that case the IRName will never be set.
> 
> When you have asm def as prevailing (gold plugin) we mark `foo` as dead in **ModuleSummaryIndex**. It's only check for
> 
>   if (AsmUndefinedRefs.count(GV.getName()))
>     return true;
>    
> 
> in thinLTOInternalizeModule which prevents DCE.

Yeah, and that's the code that we were talking separately about removing. There's even a probable current issue - if the prevailing def in the asm is marked dead, anything reachable from it might be marked dead as well (i.e. if the only use is in that def).


https://reviews.llvm.org/D41113