Reapply "IR: Remove uselist for constantdata (#137313)" This reverts commit 5936c02c8b9c6d1476f7830517781ce8b6e26e75. Fix checking uselists of constants in assume bundle queries
Possibly breaks the build: https://lab.llvm.org/buildbot/#/builders/24/builds/8119 This reverts commit 87f312aad6ede636cd2de5d18f3058bf2caf5651.
This is a resurrected version of the patch attached to this RFC:
https://discourse.llvm.org/t/rfc-constantdata-should-not-have-use-lists/42606
In this adaptation, there are a few differences. In the original patch, the Use's
use list was replaced with an unsigned* to the reference count in the value. This
version leaves them as null and leaves the ref counting only in Value.
Remove use-lists from instances of ConstantData (which are shared
across modules and have no operands).
To continue supporting most of the use-list API, store a ref-count in
place of the use-list; this is for API like Value::use_empty and
Value::hasNUses. Operations that actually need the use-list -- like
Value::use_begin -- will assert.
This change has three benefits:
1. The compiler output cannot in any way depend on the use-list order
of instances of ConstantData.
2. There's no use-list traffic when adding and removing simple
constants from operand lists (although there is ref-count traffic;
YMMV).
3. It's cheaper to serialize use-lists (since we're no longer
serializing the use-list order of things like i32 0).
The downside is that you can't look at all the users of ConstantData,
but traversals of users of i32 0 are already ill-advised.
Possible follow-ups:
- Track if an instance of a ConstantVector/ConstantArray/etc. is known
to have all ConstantData arguments, and drop the use-lists to
ref-counts in those cases. Callers need to check Value::hasUseList
before iterating through the use-list.
- Remove even the ref-counts. I'm not sure they have any benefit
besides minimizing the scope of this commit, and maintaining the
counts is not free.
Fixes #58629
Co-authored-by: Duncan P. N. Exon Smith <dexonsmith@apple.com>
Based on the output of include-what-you-use. This is a big chunk of changes. It is very likely to break downstream code unless they took a lot of care in avoiding hidden ehader dependencies, something the LLVM codebase doesn't do that well :-/ I've tried to summarize the biggest change below: - llvm/include/llvm-c/Core.h: no longer includes llvm-c/ErrorHandling.h - llvm/IR/DIBuilder.h no longer includes llvm/IR/DebugInfo.h - llvm/IR/IRBuilder.h no longer includes llvm/IR/IntrinsicInst.h - llvm/IR/LLVMRemarkStreamer.h no longer includes llvm/Support/ToolOutputFile.h - llvm/IR/LegacyPassManager.h no longer include llvm/Pass.h - llvm/IR/Type.h no longer includes llvm/ADT/SmallPtrSet.h - llvm/IR/PassManager.h no longer includes llvm/Pass.h nor llvm/Support/Debug.h And the usual count of preprocessed lines: $ clang++ -E -Iinclude -I../llvm/include ../llvm/lib/IR/*.cpp -std=c++14 -fno-rtti -fno-exceptions | wc -l before: 6400831 after: 6189948 200k lines less to process is no that bad ;-) Discourse thread on the topic: https://llvm.discourse.group/t/include-what-you-use-include-cleanup Differential Revision: https://reviews.llvm.org/D118652
The new implementation makes it clear that there are exactly two conditional stores (after the initial no-op optimization). By contrast the old implementation had seven conditionals, some hidden inside other functions. This commit can change the order of operands in operand lists, hence the tweak to one test case. Differential Revision: https://reviews.llvm.org/D80116
Summary: This patch remove waymarking and replaces it with storing a pointer to the User in the Use. here are the results on the measurements for the CTMark tests of the test suite. ``` Metric: instructions_count Program baseline patched diff test-suite :: CTMark/ClamAV/clamscan.test 72557942065 71733653521 -1.1% test-suite :: CTMark/sqlite3/sqlite3.test 76281422939 75484840636 -1.0% test-suite :: CTMark/consumer-typeset/consumer-typeset.test 51364676366 50862185614 -1.0% test-suite :: CTMark/SPASS/SPASS.test 60476106505 59908437767 -0.9% test-suite :: CTMark/tramp3d-v4/tramp3d-v4.test 112578442329 111725050856 -0.8% test-suite :: CTMark/mafft/pairlocalalign.test 50846133013 50473644539 -0.7% test-suite :: CTMark/kimwitu++/kc.test 54692641250 54349070299 -0.6% test-suite :: CTMark/7zip/7zip-benchmark.test 182216614747 181216091230 -0.5% test-suite :: CTMark/Bullet/bullet.test 123459210616 122905866767 -0.4% Geomean difference -0.8% Metric: peak_memory_use Program baseline patched diff test-suite :: CTMark/tramp3d-v4/tramp3d-v4.test 326864 338524 3.6% test-suite :: CTMark/sqlite3/sqlite3.test 216412 221240 2.2% test-suite :: CTMark/7zip/7zip-benchmark.test 11808284 12022604 1.8% test-suite :: CTMark/Bullet/bullet.test 6831752 6945988 1.7% test-suite :: CTMark/SPASS/SPASS.test 2682552 2721820 1.5% test-suite :: CTMark/ClamAV/clamscan.test 5037256 5107936 1.4% test-suite :: CTMark/consumer-typeset/consumer-typeset.test 2752728 2790768 1.4% test-suite :: CTMark/mafft/pairlocalalign.test 1517676 1537244 1.3% test-suite :: CTMark/kimwitu++/kc.test 1090748 1103448 1.2% Geomean difference 1.8% Metric: compile_time Program baseline patched diff test-suite :: CTMark/consumer-typeset/consumer-typeset.test 14.71 14.38 -2.2% test-suite :: CTMark/sqlite3/sqlite3.test 23.18 22.73 -2.0% test-suite :: CTMark/7zip/7zip-benchmark.test 57.96 56.99 -1.7% test-suite :: CTMark/ClamAV/clamscan.test 20.75 20.49 -1.2% test-suite :: CTMark/kimwitu++/kc.test 18.35 18.15 -1.1% test-suite :: CTMark/SPASS/SPASS.test 18.72 18.57 -0.8% test-suite :: CTMark/mafft/pairlocalalign.test 14.09 14.00 -0.6% test-suite :: CTMark/Bullet/bullet.test 37.38 37.19 -0.5% test-suite :: CTMark/tramp3d-v4/tramp3d-v4.test 33.81 33.76 -0.2% Geomean difference -1.1% ``` i believe that it is worth trading +1.8% peak memory use for -1.1% compile time. also this patch removes waymarking which simplifies the Use and User classes. Reviewers: nikic, lattner Reviewed By: lattner Subscribers: russell.gallop, foad, ggreif, rriddle, ekatz, fhahn, lebedev.ri, mgorny, hiraditya, george.burgess.iv, asbirlea, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D77144
to reflect the new license. We understand that people may be surprised that we're moving the header entirely to discuss the new license. We checked this carefully with the Foundation's lawyer and we believe this is the correct approach. Essentially, all code in the project is now made available by the LLVM project under our new license, so you will see that the license headers include that license only. Some of our contributors have contributed code under our old license, and accordingly, we have retained a copy of our old license notice in the top-level files in each project and repository. llvm-svn: 351636