Skip to content

Switch to new LLVM PassManager #8482

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Apr 10, 2021
Merged

Conversation

mshockwave
Copy link
Contributor

Previous attempt: #8390
Tracking issue: #8418

Previously we tried to switch to new LLVM PassManager in llvm12 branch, but reverted back due to two outstanding problems:

  1. A crash related to @setEvalBranchQuota
  2. Slow compilation speed ( > 2x slower )

It turned out these two problems were caused by building the -O0 Pass pipeline in a wrong way. This PR -- which is nearly identical to #8390 -- addressed this problem and try to bring new LLVM PassManager back to zig again.
I also removed two redundant Passes, which were added by accident, in the LTO pipeline.

So far this PR passes all the tests except those who are already failing in the llvm12 branch.

Use `PassBuilder::buildO0DefaultPipeline` to build pipeline for -O0
in replacement of `PassBuilder::buildPerModuleDefaultPipeline`. This
affects both normal and LTO settings.

Two redundant Passes - which were added by accident - were also removed
from LTO pipeline.
@andrewrk andrewrk merged commit 86b3139 into ziglang:llvm12 Apr 10, 2021
@mshockwave
Copy link
Contributor Author

I tried to run all the tests again after this patched is applied and found that one test case failed:

$ zig build test-stack-traces
...
Test 10/10 stack-trace dumpCurrentStackTrace (Debug)
========= Expected this output: =========
source.zig:7:8: [address] in foo (test)
    bar();
       ^
source.zig:10:8: [address] in main (test)
    foo();
       ^
start.zig:342:29: [address] in std.start.posixCallMainAndExit (test)
            return root.main();
                            ^
start.zig:163:5: [address] in std.start._start (test)
    @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{});
================================================
source.zig:7:8: [address] in foo (test)
    bar();
       ^
source.zig:10:8: [address] in main (test)
    foo();
       ^
start.zig:342:29: [address] in std.start.callMain (test)
            return root.main();
                            ^
start.zig:287:12: [address] in std.start.callMainWithArgs (test)
    return @call(.{ .modifier = .always_inline }, callMain, .{});
           ^
start.zig:241:17: [address] in std.start.posixCallMainAndExit (test)
    std.os.exit(@call(.{ .modifier = .always_inline }, callMainWithArgs, .{ argc, argv, envp }));
                ^
start.zig:163:5: [address] in std.start._start (test)
    @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{});

It turns out I didn't fully clear the cache folder before running the tests prior to submitting this PR, which gives me a false positive that I passed this specific test.

I sincerely apologize for this mistake. I will fix it.

If keep reverting the commit brings too many burdens, maybe we can put this as a long term project and put it as a separate branch. After all, migrating to a new PassManager has never been an easy task (Although this specific test failure is kind of minor and I don't think it will be a blocker).

Again, sorry for this mistake

@mikdusan
Copy link
Member

mikdusan commented Apr 10, 2021

actually, I just began an overhaul of stack_trace tests and should have something shortly to address test sensitivity to platform and optimizer effects.

@andrewrk
Copy link
Member

Not to worry @mshockwave , thanks for your excellent work!

@mshockwave
Copy link
Contributor Author

actually, I just began an overhaul of stack_trace tests and should have something shortly to address test sensitivity to platform and optimizer effects.

I believe it will be helpful.
The issue here is caused by the fact that AlwaysInliner pass has a quite different behaviors in legacy and new PM: In legacy PM, AlwaysInliner tries to inline functions (declarations) AND callsites with alwaysinline attributes. On the contrary, AlwaysInliner in new PM only inlines functions with such attribute. More specifically, on this problematic callsite:

std.os.exit(@call(.{ .modifier = .always_inline }, callMainWithArgs, .{ argc, argv, envp }));

This callsite was marked alwaysinline in LLVM IR but the callee function callMainWithArgs was not.

If we want to inline callsite -- regardless the existence of alwaysinline -- in new PM, we need to use the normal inliner Pass, which is quite heavy in terms of compilation time.

@andrewrk I remembered you mentioned before that after we have a complete self-hosted compiler, we're doing always-inlining in the self-hosted part rather than LLVM right? One possible workaround will be write our own simple inliner Pass that replicate the behavior of legacy AlwaysInliner Pass, which will be removed after the self-hosted always-inliner is complete. What do you think?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants