[PATCH] D95970: [HIP] Allow undefined symbols

Fri Feb 5 20:10:48 PST 2021

yaxunl abandoned this revision.
yaxunl added a comment.

In D95970#2540669 <https://reviews.llvm.org/D95970#2540669>, @tra wrote:

> In D95970#2540414 <https://reviews.llvm.org/D95970#2540414>, @yaxunl wrote:
>
>> In D95970#2540303 <https://reviews.llvm.org/D95970#2540303>, @tra wrote:
>>
>>> What's going to happen if you do have an undefined reference that's *not* to a `__managed__` variable?
>>
>> By default HIP toolchain uses -fvisibility hidden -fapply-global-visibility-to-externs for device compilation. Undefined variable symbols have protected visibility. Undefined function symbols have hidden visibility. Since they are not allowed to be preempted, lld still emits error if they are undefined. `__managed__` variables have default visibility, therefore they are allowed to go through.
>
> Is there a test that verifies/demonstrates how it's handled at compile time?
>
>> We can let HIP runtime emit an error if there are undefined symbols other than managed variables.
>
> Detecting compilation errors at run-time is not very useful. The end user would likely have no idea what's going on.
> I wonder if we can catch the errors earlier. E.g. w/o RDC, we can probably catch it in the codegen.
> With RDC, only the linker will know it, so you would need some sort of custom plugin for that.
>
> What if we actually resolve all `__managed__` references and point them to some sort of placeholder. 
> E.g a weak symbol, that would be directed to the real location when the binary is loaded. 
> Or, maybe, introduce an indirection and let the runtime handle managed variables as a special case 
> and point the indirect links to the right locations. 
> This way you can still run with `-no-undefined` and find all other unresolved references that should not be there.

Good point. I will try using indirection. This should avoid undefined symbols in device binary.

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

https://reviews.llvm.org/D95970