Page MenuHomePhabricator

[Attributor] Add an Attributor CG-SCC pass
AcceptedPublic

Authored by jdoerfert on Nov 26 2019, 11:35 PM.

Details

Summary
In addition to the module pass, this patch introduces a CG-SCC pass that                                                                                                                                                              
runs the Attributor on a strongly connected component of the call graph
(both old and new PM). The Attributor was always design to be used on a
subset of functions which makes this patch mostly mechanical.

The one change is that we give up `norecurse` deduction in the module
pass in favor of doing it during the CG-SCC pass. This makes an
interfaces simpler but can be revisited.

Diff Detail

Unit TestsFailed

TimeTest
410 msLLVM-Unit.Analysis/_/AnalysisTests::CGSCCPassManagerTest.TestUpdateCGAndAnalysisManagerForPasses8
Note: Google Test filter = CGSCCPassManagerTest.TestUpdateCGAndAnalysisManagerForPasses8 [==========] Running 1 test from 1 test case. [----------] Global test environment set-up.
100 msLLVM.Other::new-pm-thinlto-postlink-pgo-defaults.ll
Script: -- : 'RUN: at line 4'; /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/opt -disable-verify -debug-pass-manager -passes='thinlto<O1>' -S /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll 2>&1 | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/FileCheck /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll --check-prefixes=CHECK-O,CHECK-O1,CHECK-NOEXT --dump-input=fail
100 msLLVM.Other::new-pm-thinlto-postlink-samplepgo-defaults.ll
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/opt -disable-verify -debug-pass-manager -pgo-kind=pgo-sample-use-pipeline -profile-file='/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/Inputs/new-pm-thinlto-samplepgo-defaults.prof' -passes='thinlto<O1>' -S /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll 2>&1 | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/FileCheck /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll --check-prefixes=CHECK-O,CHECK-O1,CHECK-NOEXT --dump-input=fail
80 msLLVM.Other::new-pm-thinlto-prelink-pgo-defaults.ll
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/llvm-profdata merge /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/Inputs/new-pm-thinlto-prelink-pgo-defaults.proftext -o /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Other/Output/new-pm-thinlto-prelink-pgo-defaults.ll.tmp.profdata
90 msLLVM.Other::new-pm-thinlto-prelink-samplepgo-defaults.ll
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/opt -disable-verify -debug-pass-manager -pgo-kind=pgo-sample-use-pipeline -profile-file='/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/Inputs/new-pm-thinlto-samplepgo-defaults.prof' -passes='thinlto-pre-link<O1>,name-anon-globals' -S /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll 2>&1 | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/FileCheck /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll --check-prefixes=CHECK-O,CHECK-O1,CHECK-O-NODIS,CHECK-O123 --dump-input=fail
View Full Test Results (19 Failed)

Event Timeline

jdoerfert created this revision.Nov 26 2019, 11:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 26 2019, 11:35 PM
jdoerfert updated this revision to Diff 231785.EditedDec 2 2019, 2:51 PM

Use the CallGraphUpdater (D70927) to leep the call graph up to date

Build result: FAILURE - Could not check out parent git hash "548fb3a7879311b94ef69aed9747cb8b6ef92374". It was not found in the repository. Did you configure the "Parent Revision" in Phabricator properly? Trying to apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt, CMakeCache.txt

jdoerfert retitled this revision from [Attributor] Add an Attributor CG-SCC pass NOTE: Not completely finished, see TODOs below! to [Attributor] Add an Attributor CG-SCC pass.Dec 9 2019, 12:08 AM
jdoerfert edited the summary of this revision. (Show Details)
jdoerfert updated this revision to Diff 232764.Dec 9 2019, 12:09 AM
jdoerfert edited the summary of this revision. (Show Details)

Small improvements + updated (=passing) tests

Build result: FAILURE - Could not check out parent git hash "7af3744fd9261d018af1ab25e2147d81505328ca". It was not found in the repository. Did you configure the "Parent Revision" in Phabricator properly? Trying to apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt, CMakeCache.txt

Can you add a testcase from https://reviews.llvm.org/D70737, please?

I think changes for replaceAllUsesWith are not related to CG-SCC pass. For that part, LGTM. Could you split the patch?

llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
529

Do you have any reason why you move Attributor here?

jdoerfert marked an inline comment as done.Dec 11 2019, 11:58 AM

Can you add a testcase from https://reviews.llvm.org/D70737, please?

Will do.

I think changes for replaceAllUsesWith are not related to CG-SCC pass. For that part, LGTM. Could you split the patch?

Will do.

llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
529

The module pass should run early to improve the passes before the inliner loop, e.g., createCalledValuePropagationPass below.

jdoerfert updated this revision to Diff 233915.Dec 13 2019, 9:24 PM

Split out replaceAllUsesWith, added D70737 test case

Unit tests: fail. 60890 tests passed, 8 failed and 726 were skipped.

failed: LLVM.Other/new-pm-defaults.ll
failed: LLVM.Other/new-pm-thinlto-defaults.ll
failed: LLVM.Other/opt-O2-pipeline.ll
failed: LLVM.Other/opt-O3-pipeline.ll
failed: LLVM.Other/opt-Os-pipeline.ll
failed: LLVM.Other/pass-pipelines.ll
failed: LLVM.Transforms/Attributor/IPConstantProp/pthreads.ll
failed: LLVM.Transforms/Attributor/IPConstantProp/return-constant.ll

clang-format: pass.

Build artifacts: console-log.txt, CMakeCache.txt, test-results.xml, diff.json

Adjust the pass manager tests

Unit tests: pass. 60898 tests passed, 0 failed and 726 were skipped.

clang-format: pass.

Build artifacts: console-log.txt, CMakeCache.txt, test-results.xml, diff.json

hfinkel accepted this revision.Dec 18 2019, 5:16 AM
hfinkel added a subscriber: hfinkel.

LGTM

This revision is now accepted and ready to land.Dec 18 2019, 5:16 AM
xbolva00 added inline comments.Dec 18 2019, 6:29 AM
llvm/include/llvm/LinkAllPasses.h
196

Space

jdoerfert updated this revision to Diff 236475.Mon, Jan 6, 3:27 PM

Rebase and better use of D72025

Unit tests: fail. 61272 tests passed, 6 failed and 736 were skipped.

failed: LLVM.Other/new-pm-defaults.ll
failed: LLVM.Other/new-pm-thinlto-defaults.ll
failed: LLVM.Other/opt-O2-pipeline.ll
failed: LLVM.Other/opt-O3-pipeline.ll
failed: LLVM.Other/opt-Os-pipeline.ll
failed: LLVM.Other/pass-pipelines.ll

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

jdoerfert updated this revision to Diff 236601.Tue, Jan 7, 8:38 AM
jdoerfert marked 2 inline comments as done.

Update tests and address comment

Unit tests: fail. 61301 tests passed, 11 failed and 736 were skipped.

failed: LLVM.Transforms/Attributor/ArgumentPromotion/crash.ll
failed: LLVM.Transforms/Attributor/ArgumentPromotion/fp80.ll
failed: LLVM.Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll
failed: LLVM.Transforms/Attributor/IPConstantProp/PR26044.ll
failed: LLVM.Transforms/Attributor/IPConstantProp/fp-bc-icmp-const-fold.ll
failed: LLVM.Transforms/Attributor/IPConstantProp/recursion.ll
failed: LLVM.Transforms/Attributor/IPConstantProp/solve-after-each-resolving-undefs-for-function.ll
failed: LLVM.Transforms/Attributor/liveness.ll
failed: LLVM.Transforms/Attributor/noreturn_async.ll
failed: LLVM.Transforms/Attributor/noreturn_sync.ll
failed: LLVM.Transforms/Attributor/undefined_behavior.ll

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Unit tests: fail. 62144 tests passed, 19 failed and 811 were skipped.

failed: LLVM-Unit.Analysis/_/AnalysisTests/CGSCCPassManagerTest.TestUpdateCGAndAnalysisManagerForPasses8
failed: LLVM.Other/new-pm-thinlto-postlink-pgo-defaults.ll
failed: LLVM.Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
failed: LLVM.Other/new-pm-thinlto-prelink-pgo-defaults.ll
failed: LLVM.Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
failed: LLVM.Transforms/Attributor/ArgumentPromotion/dbg.ll
failed: LLVM.Transforms/Attributor/ArgumentPromotion/live_called_from_dead.ll
failed: LLVM.Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll
failed: LLVM.Transforms/Attributor/ArgumentPromotion/pr27568.ll
failed: LLVM.Transforms/Attributor/ArgumentPromotion/reserve-tbaa.ll
failed: LLVM.Transforms/Attributor/IPConstantProp/pthreads.ll
failed: LLVM.Transforms/Attributor/liveness.ll
failed: LLVM.Transforms/Attributor/noalias.ll
failed: LLVM.Transforms/Attributor/range.ll
failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 2 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.