llvm dev - Oct 2017 - Reducing confusion around isUndefined()

(switching list from llvm-commits to llvm-dev, which seems more appropriate
here)

From: llvm-commits <llvm-commits-bounces at lists.llvm.org> on behalf of
Rui Ueyama via llvm-commits <llvm-commits at lists.llvm.org>
Reply-To: Rui Ueyama <ruiu at google.com>
Date: Friday, October 13, 2017 at 4:22 PM
To: Rafael Avila de Espindola <rafael.espindola at gmail.com>
Cc: llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: Reducing confusion around isUndefined()

I think I'm not too excited about adding more predicates or inheritance.
Lazy symbols are useful only when we are adding files to the symbol table. After
that, there's no use of them. So maybe we can just visit all symbols at some
point after reading all files but before writeResult to nuke all Lazy symbols
(by replacing local undefined symbol or something), so that we don't have to
think about it after that point.

On Fri, Oct 13, 2017 at 3:27 PM, Rafael Avila de Espindola <rafael.espindola
at gmail.com<mailto:rafael.espindola at gmail.com>> wrote:
In lld we have three types of symbols that are easily confused:
undefined, shared and lazy.

I just finished auditing the uses of isUndefined, and most were
incorrect.

The most common issue was that the code intended to check if the symbol
will be undefined on the final symbol table. The actual predicate for
that is isInCurrentDSO().

In some other cases the intention was to check if we had a definition
already, in which case the code should check also for lazy symbols
too. We don't currently have a predicate for isUndefined() || isLazy().

Some ideas to try to make this less error prone.

* Add a isNotDefined() predicate for isUndefined() ||
  isLazy(). Unfortunately the name would be confusing.

* Have Lazy inherit from undefined. Very few places want to check just
  for undefined, so they can use kind() directly and isUndefined() would
  also return true for lazy symbols.

* Rename isUndefined() to isUndefinedOnInput() to make it clear it is
  not about the output symbol table.

I am not too excited about any of these. Anyone has another suggestion
on how to make this more intuitive?

Cheers,
Rafael

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171014/bee34d0d/attachment.html>

Shoaib Meenai <smeenai at fb.com> writes:
> (switching list from llvm-commits to llvm-dev, which seems more appropriate
here)
Thanks.
> I think I'm not too excited about adding more predicates or
inheritance. Lazy symbols are useful only when we are adding files to the symbol
table. After that, there's no use of them. So maybe we can just visit all
symbols at some point after reading all files but before writeResult to nuke all
Lazy symbols (by replacing local undefined symbol or something), so that we
don't have to think about it after that point.
True. We should be able to either remove or convert to undef every lazy
symbol after we read all the files.

Cheers,
Rafael

I ran into this a couple of months ago, when doing something else, but was
too caught up in that so didn't get around to raising it. I agree that
converting the symbols from lazy after we're done with them makes sense. It
would also allow us to simplify a number of places where we check for !Lazy
&& !Undefined or similar.

On 14 October 2017 at 07:03, Rafael Avila de Espindola via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Shoaib Meenai <smeenai at fb.com> writes:
>
> > (switching list from llvm-commits to llvm-dev, which seems more
> appropriate here)
> Thanks.
>
> > I think I'm not too excited about adding more predicates or
inheritance.
> Lazy symbols are useful only when we are adding files to the symbol table.
> After that, there's no use of them. So maybe we can just visit all
symbols
> at some point after reading all files but before writeResult to nuke all
> Lazy symbols (by replacing local undefined symbol or something), so that we
> don't have to think about it after that point.
>
> True. We should be able to either remove or convert to undef every lazy
> symbol after we read all the files.
>
> Cheers,
> Rafael
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171020/b317d77a/attachment.html>