Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

800x compile time regression for AVR.cpp on sparcv9 #48442

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR49473
Status NEW
Importance P normal
Reported by Rainer Orth (ro@gcc.gnu.org)
Reported on 2021-03-08 04:52:53 -0800
Last modified on 2021-03-09 09:24:50 -0800
Version trunk
Hardware Sun Solaris
CC aeubanks@google.com, dblaikie@gmail.com, htmldeveloper@gmail.com, jrtc27@jrtc27.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments AVR.llvm13 (27409 bytes, text/plain)
AVR.json.bz2 (67284 bytes, application/x-bzip)
Blocks
Blocked by
See also
Created attachment 24601
-ftime report output

The compile time for clang/lib/Driver/ToolChains/AVR.cpp on Solaris/sparcv9 has
exploded recently: it grew from 4.92s to 1h 3m 50.16s, almost a factor of 800
and doubling the wall clock time of ninja -j96 on a Netra SPARC S7-2.

A reghunt identified the new PM as the culprit

commit 669ddd1e9b1226432b003dbba05b99f8e992285b
Author: Arthur Eubanks <aeubanks@google.com>
Date:   Mon Jan 25 11:00:56 2021 -0800

    Turn on the new pass manager by default

and indeed compile time is back to normal with -flegacy-pass-manager.

Unfortunately, a build with -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=OFF fails:

FAILED:
tools/clang/tools/extra/clangd/CMakeFiles/obj.clangDaemon.dir/ClangdLSPServer.cpp.o
[...]
Did not see access type in access path!
  store i64 %Handler.coerce1, i64* %.fca.1.gep, align 8, !tbaa !372
!372 = !{!370, !6, i64 8}

I'm attaching the -ftime-report output for reference.
Quuxplusone commented 3 years ago

Attached AVR.llvm13 (27409 bytes, text/plain): -ftime report output

Quuxplusone commented 3 years ago
FWIW, if I omit clang-tools-extra from the list of projects to build (i.e.
restrict
it to clang;compiler-rt;polly, a 2-stage build with -
DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER=OFF
completes.

Besides, there are some test result differences with and without the new PM:

-Failed Tests (75):
+Failed Tests (72):
+  LLVM-Unit :: BinaryFormat/./BinaryFormatTests/MachOTest.UnalignedLC
   LLVM :: Bindings/Go/go.test
-  LLVM :: CodeGen/X86/known-signbits-vector.ll
-  LLVM :: CodeGen/X86/vec_shift5.ll
-  LLVM :: CodeGen/X86/vector-shift-ashr-128.ll
-  LLVM :: CodeGen/X86/vector-shift-ashr-256.ll
Quuxplusone commented 3 years ago

Could you attach -ftime-trace?

Also, is your clang compiled with asserts on? If so have you synced to https://reviews.llvm.org/D97225, which disables some checks that can take a long time?

Quuxplusone commented 3 years ago

Attached AVR.json.bz2 (67284 bytes, application/x-bzip): -ftime-trace output

Quuxplusone commented 3 years ago
(In reply to Arthur Eubanks from comment #2)
> Could you attach -ftime-trace?

Sure, done.

> Also, is your clang compiled with asserts on? If so have you synced to

It is, to match what the buildbots use.

> https://reviews.llvm.org/D97225, which disables some checks that can take a
> long time?

I had indeed.
Quuxplusone commented 3 years ago

Ok so it's not related to those checks then.

If I'm reading the time trace right (loadable via chrome://tracing), it seems to show that isel is taking a long time, rather than any specific middle-end pass.

If I had to guess, SPARC isel has really bad runtimes on specific inputs, e.g. large functions. The new PM has a different inliner and can cause some functions to be large. You can check the LLVM IR to see if any functions are much larger with the legacy PM vs the new PM (clang++ -Xclang -disable-llvm-passes -emit-llvm -S). So I don't think this is a new PM issue, merely that it exposes an issue with SPARC isel. Adding people who have worked on that would be good.

And now I see that I missed the instruction selection section in the initial -ftime-report that showed this.

Quuxplusone commented 3 years ago
(In reply to Arthur Eubanks from comment #5)

> If I had to guess, SPARC isel has really bad runtimes on specific inputs,
> e.g. large functions. The new PM has a different inliner and can cause some
> functions to be large. You can check the LLVM IR to see if any functions are
> much larger with the legacy PM vs the new PM (clang++ -Xclang
> -disable-llvm-passes -emit-llvm -S). So I don't think this is a new PM

I tried this, but AVR.ll is identical without and with the new PM.

> issue, merely that it exposes an issue with SPARC isel. Adding people who
> have worked on that would be good.

That might be hard, unfortunately: many changes to the Sparc backend have been
purely mechanical recently.  I'm trying Jessica, but two others don't even have
a bugzilla account, unfortunately (LemonBoy <thatlemon@gmail.com> and Daniel
Cederman <cederman@gaisler.com>).
Quuxplusone commented 3 years ago

Sorry, I didn't mean to add -Xclang -disable-llvm-passes to the command line, that disables running any passes at all on the output of clang.

It should be clang++ -S -emit-llvm -flegacy-pass-manager vs clang++ -S -emit-llvm -fno-legacy-pass-manager.

There should be a difference there.