Ticket #1777 (closed bug: fixed)

Opened 4 years ago

Last modified 4 years ago

Optimized builds (--optimize='-O2 -finline-functions') on gcc/amd64 broken since r48308

Reported by: doughera Owned by:
Priority: normal Milestone:
Component: none Version: trunk
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

Currently, as of r48867, optimized builds with gcc on amd64 don't work. Specifically, the build ends with

./parrot runtime/parrot/library/PGE/Perl6Grammar.pir --output=compilers/pge/PGE/builtins_gen.pir compilers/pge/PGE/builtins.pg
Null PMC access in thaw()
current instr.: 'parrot;PGE;Perl6Grammar;Compiler;__onload' pc 24 (runtime/parrot/library/PGE/Perl6Grammar.pir:75)
make: *** [runtime/parrot/library/PGE.pbc] Error 1

This is with gcc version 4.3.2 (Debian 4.3.2-1.1), and --optimize=-O4.

Trying to trace it back, the most recent version on which I could successfully build was r48307. I have tried 142 different trunk revisions between r48308 and r48867, and none of them built successfully. The errors weren't all exactly the same, but in none of them did the build complete. I have not had time to look at r48308 carefully.

Attachments

tt1777-avoid-inline.patch Download (468 bytes) - added by doughera 4 years ago.
Avoid inlining PF_fetch_opcode

Change History

follow-up: ↓ 2   Changed 4 years ago by mikehh

I have not had this problem and I have build with gcc/g++ with or without --optimize on ubuntu 10.04 amd64 and i386. This uses gcc/g++ version 4.4.3 and I think the standard --optimize is -O2.

I have never attempted to use higher levels of --optimize.

Will see what happens

Cheers Michael (mikehh)

in reply to: ↑ 1   Changed 4 years ago by doughera

Replying to mikehh:

I have not had this problem and I have build with gcc/g++ with or without --optimize on ubuntu 10.04 amd64 and i386. This uses gcc/g++ version 4.4.3 and I think the standard --optimize is -O2.

The "standard" for --optimize is whatever the underlying perl happened to be compiled with. Rakudo uses --optimize, and my perl happened to be one I compiled myself with -O4. That's how I stumbled upon this.

For gcc-4.3.2, r48308 passes with -O2, but fails with -O3.

I have never attempted to use higher levels of --optimize. Will see what happens

Thanks. Independent confirmation would be nice.

Changed 4 years ago by doughera

Avoid inlining PF_fetch_opcode

follow-up: ↓ 5   Changed 4 years ago by doughera

This appears to result from gcc inlining functions, and patch r48308 just seems to have pushed something over some threshold. If I force gcc not to inline src/packfile/pf_items.c:PF_fetch_opcode, then r48308 builds. A patch is attached. It probably could be made more specific to gcc versions and architectures.

With this patch, r48308 builds, but it still fails the following tests:

Failed Test                   Stat Wstat Total Fail  List of Failed
-------------------------------------------------------------------------------
t/pmc/packfile.t                            37    9  12-16 18-20 35
t/pmc/packfileannotations.t                 17    1  2
t/pmc/packfileconstanttable.t               16    3  1-3
t/pmc/packfiledirectory.t                   20    5  5-6 18-20
t/pmc/packfilefixupentry.t                   3    3  1-3
t/pmc/packfilefixuptable.t                   3    1  1
t/pmc/packfilerawsegment.t                   7    3  1-2 5

Current parrot (r48882) still doesn't build; that may be a different problem.

follow-up: ↓ 6   Changed 4 years ago by NotFound

The patch doesn't fix the build for me in Debian amd64 gcc 4.3.2 --optimize

in reply to: ↑ 3   Changed 4 years ago by plobsing

Replying to doughera:

This appears to result from gcc inlining functions, and patch r48308 just seems to have pushed something over some threshold. If I force gcc not to inline src/packfile/pf_items.c:PF_fetch_opcode, then r48308 builds. A patch is attached. It probably could be made more specific to gcc versions and architectures. With this patch, r48308 builds, but it still fails the following tests:

Failed Test                   Stat Wstat Total Fail  List of Failed
-------------------------------------------------------------------------------
t/pmc/packfile.t                            37    9  12-16 18-20 35
t/pmc/packfileannotations.t                 17    1  2
t/pmc/packfileconstanttable.t               16    3  1-3
t/pmc/packfiledirectory.t                   20    5  5-6 18-20
t/pmc/packfilefixupentry.t                   3    3  1-3
t/pmc/packfilefixuptable.t                   3    1  1
t/pmc/packfilerawsegment.t                   7    3  1-2 5

Current parrot (r48882) still doesn't build; that may be a different problem.

Those failing tests are to be expected - the test pbcs needed updating (which happened in r48309).

in reply to: ↑ 4   Changed 4 years ago by doughera

Replying to NotFound:

The patch doesn't fix the build for me in Debian amd64 gcc 4.3.2 --optimize

You don't specify, but I assume you mean it still doesn't build in r48882 (or later). That's consistent with what I see.

Further followups: r48309 (together with the noinline band-aid patch) fixes the packfile test failures I mentioned above. Trying to work backwards from the current failures, the problems originate with the merge of the charset_massacre branch. Working backwards on the charset_massacre branch, with my noinline bandaid, and a manual patch around the Uchar32 issue in ucs4.c, the current breakage appears to start at r48796 which, unfortunately, is simply a big merge affecting 32 files to Bring branch up-to-date with trunk.

follow-up: ↓ 10   Changed 4 years ago by NotFound

There was a problem with a missing return during some revisions, this can interfere with the dissects.

  Changed 4 years ago by NotFound

More specific; missing return introduced in r48804, fixed in r48820

  Changed 4 years ago by nwellnhof

If I had to guess which parts of r48796 cause the breakage, I'd say it's r48783 or r48784.

in reply to: ↑ 7   Changed 4 years ago by doughera

Replying to NotFound:

There was a problem with a missing return during some revisions, this can interfere with the dissects.

Yes, that sort of thing is why I quickly gave up on bisecting. Instead, I just had a script sample random revisions inside the relevant range.

More generally, I don't think inlining itself is the direct cause, but is rearranging stuff sufficiently that it is tickling a problem elsewhere. Unfortunately, that's the sort of thing that requires thinking, rather than automated unattended random revision testing, so I don't have time to pursue it further for now.

  Changed 4 years ago by NotFound

Committed a change in r48897 that builds and pass tests for me in amd64 --optimize with gcc 4.3.2

StringBuilder was doing its own memory management of its STRING buffer. This change switches to the specialized string memory functions.

  Changed 4 years ago by bacek

Hello.

Originally StringBuilder uses non GCables STRINGS for performance reason. StringPool compacting happens too often.

-- Bacek

  Changed 4 years ago by NotFound

Losing memory in use is a price too high to pay.

  Changed 4 years ago by doughera

  • summary changed from Optimized builds on gcc/amd64 broken since r48308 to Optimized builds (--optimize='-O2 -finline-functions') on gcc/amd64 broken since r48308

I have changed the description to more accurately reflect the relevant optimization flags. In particular, it seems to be -finline-functions (included automatically in -O3 or higher) that is the relevant flag.

  Changed 4 years ago by NotFound

plobsing++ fixed PF_fetch_opcode, which was failing in highly optimized builds and causing the remaining errors. After that fix, we've been able to build, pass tests and run big libraries and languages with -O3 and with -O2 -finline-functions.

Closing ticket.

follow-up: ↓ 17   Changed 4 years ago by NotFound

Forgot to check the resolve button... But kid51 suggested leaving open for a few days just in case.

in reply to: ↑ 16   Changed 4 years ago by doughera

  • status changed from new to closed
  • resolution set to fixed

Replying to NotFound:

Forgot to check the resolve button... But kid51 suggested leaving open for a few days just in case.

The change looks very sensible. (Curiously, turning on gcc's -fno-strict-aliasing also fixed the problem. I tried upgrading to gcc-4.5.1 to see what it would do, but that had multiple build failures, and I eventually gave up on it.) Unfortunately, when I tried to test the current revision of parrot myself, I got unrelated build failures. When I have a chance, I'll try to track them down.

Meanwhile, I'll close this ticket. Thanks for the fix.

Note: See TracTickets for help on using tickets.