Bug 61635 - LTO partitioner does not handle &&label in statics
Summary: LTO partitioner does not handle &&label in statics
Status: RESOLVED DUPLICATE of bug 50676
Alias: None
Product: gcc
Classification: Unclassified
Component: lto (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: 5.0
Assignee: Jan Hubicka
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-27 21:26 UTC by Andi Kleen
Modified: 2015-03-30 07:29 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-06-27 00:00:00


Attachments
lto.config (18.45 KB, text/plain)
2014-06-28 01:16 UTC, andi
Details
partitioner (2.31 KB, text/plain)
2015-03-30 04:36 UTC, Jan Hubicka
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andi Kleen 2014-06-27 21:26:48 UTC
I currently don't have a small compilable test case, except a build tree from a large project. But what happened was code like this

f()
{
   static void *addr[] = {
          &&label1,
          &&label2, 
          ..
   };
   /* labels defined in the code */
}

The LTO partitioner would put addr into a different ltrans unit than f.
But the assembler output of addr contains direct references to the code labels in the assembler of f.

This results in lots of assembler errors for undefined labels.
Comment 1 Jan Hubicka 2014-06-27 23:04:00 UTC
Yep, this is a known problem. We do not represent labels in symbol table and thus LTO partitioner has no clue about them. I would be interested to see the example that fails - theoretically the problematic static variable should always end up in the same partition as the function using it, because that function is no inline and no clone and should not get duplicated. So kernel & friends should fail only with -flto-partition=1to1 not with ballanced/none/one algorithms.

I plan correct solution - I already implemented labels for symtab some time ago but I would like to re-do it on the new API.
The poor-man's class hiearchy in C was not very pretty and a lot of code is still not updated to nots assume two basic types of symbols (variables and functions). With Martin Liska we are updating the APIs and once it is done I will add symbols for non-local labels and const_decls.
Comment 2 andi 2014-06-28 01:15:56 UTC
Created attachment 33023 [details]
lto.config

Test case

git clone https://github.com/andikleen/linux-misc -b lto-linus-3.15
Build with the attached kernel config (copy to .config in the build dir) and 4.9

-Andi

On Fri, Jun 27, 2014 at 11:04:00PM +0000, hubicka at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61635
> 
> Jan Hubicka <hubicka at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>              Status|UNCONFIRMED                 |ASSIGNED
>    Last reconfirmed|                            |2014-06-27
>                  CC|                            |hubicka at gcc dot gnu.org
>            Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org
>      Ever confirmed|0                           |1
> 
> --- Comment #1 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
> Yep, this is a known problem. We do not represent labels in symbol table and
> thus LTO partitioner has no clue about them. I would be interested to see the
> example that fails - theoretically the problematic static variable should
> always end up in the same partition as the function using it, because that
> function is no inline and no clone and should not get duplicated. So kernel &
> friends should fail only with -flto-partition=1to1 not with ballanced/none/one
> algorithms.
> 
> I plan correct solution - I already implemented labels for symtab some time ago
> but I would like to re-do it on the new API.
> The poor-man's class hiearchy in C was not very pretty and a lot of code is
> still not updated to nots assume two basic types of symbols (variables and
> functions). With Martin Liska we are updating the APIs and once it is done I
> will add symbols for non-local labels and const_decls.
> 
> -- 
> You are receiving this mail because:
> You reported the bug.
>
Comment 3 Jan Hubicka 2014-06-28 06:33:24 UTC
> 
> git clone https://github.com/andikleen/linux-misc -b lto-linus-3.15
> Build with the attached kernel config (copy to .config in the build dir) and
> 4.9

Aha, we still compile kernel with -fno-toplevel-reporder? That probably could drag
variables out of their contextes.  Will have to take a look.

Honza
Comment 4 Andi Kleen 2014-06-28 15:51:30 UTC
Yes it uses -fno-toplevel-reordering to avoid the problems with the initializer reordering.

I tried some workarounds for this, but nothing worked so far. Likely would need
a noreorder attribute.
Comment 5 Andi Kleen 2014-06-28 15:57:15 UTC
Also I forgot to state: the git tree above now has a workaround
(disabling LTO for that file). If you want to reproduce revert the latest commit first.
Comment 6 Jan Hubicka 2014-06-28 21:54:57 UTC
> Yes it uses -fno-toplevel-reordering to avoid the problems with the initializer
> reordering.
> 
> I tried some workarounds for this, but nothing worked so far. Likely would need
> a noreorder attribute.

Actually I think other chance to make this to fail is when the address/value of the local static
is propagated by ipa-prop into some extra function...

Perhaps as a workaround in 4.9 we may disable such a propagation.
Honza
Comment 7 Andi Kleen 2015-03-29 18:24:21 UTC
Still happens with current trunk and with newer LTO Linux kernels (4.0-rc*)
Comment 8 Jan Hubicka 2015-03-30 04:05:53 UTC
I am testing fix that is small enough for GCC 5 (and backportable to release branches I guess)
Comment 9 Jan Hubicka 2015-03-30 04:36:47 UTC
Created attachment 35177 [details]
partitioner

This is patch I am testing
Comment 10 Jan Hubicka 2015-03-30 07:29:04 UTC
We already have ealrier PR for this.

*** This bug has been marked as a duplicate of bug 50676 ***