The llvm.metadata section is not emitted and has special semantics. We
should not merge globals in it, similarly to how we already skip merging
of `llvm.xyz` globals.
Fixes https://github.com/llvm/llvm-project/issues/131394.
For the NewPM, the merge-const option was assigned to an unused
option field. Assign it to the correct one. The merge-const-aggressive
option was not supported -- and invalid options were silently ignored.
Accept it and error on invalid options.
For the LegacyPM, the corresponding cl::opt options were ignored when
called via opt rather than llc.
The module currently stores the target triple as a string. This means
that any code that wants to actually use the triple first has to
instantiate a Triple, which is somewhat expensive. The change in #121652
caused a moderate compile-time regression due to this. While it would be
easy enough to work around, I think that architecturally, it makes more
sense to store the parsed Triple in the module, so that it can always be
directly queried.
For this change, I've opted not to add any magic conversions between
std::string and Triple for backwards-compatibilty purses, and instead
write out needed Triple()s or str()s explicitly. This is because I think
a decent number of them should be changed to work on Triple as well, to
avoid unnecessary conversions back and forth.
The only interesting part in this patch is that the default triple is
Triple("") instead of Triple() to preserve existing behavior. The former
defaults to using the ELF object format instead of unknown object
format. We should fix that as well.
Reverts llvm/llvm-project#124146 -- new comparator is not a strict-weak
as required by stable_sort.
Co-authored-by: Michael Maitland <michaeltmaitland@gmail.com>
We were previously sorting by profitability even if we were choosing to
merge all globals together, which is not impacted by UsedGlobalSet
order.
We can also remove iteration of UsedGlobalSets in reverse order in both
cases. In the first csae, the order does not matter. In the second case,
we just sort by the order we need instead of sorting in the opposite
direction and calling reverse.
This change should only be an improvement on compile time. I have not
measured it, but I think it would never make things worse.
As part of the "RemoveDIs" project, BasicBlock::iterator now carries a
debug-info bit that's needed when getFirstNonPHI and similar feed into
instruction insertion positions. Call-sites where that's necessary were
updated a year ago; but to ensure some type safety however, we'd like to
have all calls to getFirstNonPHI use the iterator-returning version.
This patch changes a bunch of call-sites calling getFirstNonPHI to use
getFirstNonPHIIt, which returns an iterator. All these call sites are
where it's obviously safe to fetch the iterator then dereference it. A
follow-up patch will contain less-obviously-safe changes.
We'll eventually deprecate and remove the instruction-pointer
getFirstNonPHI, but not before adding concise documentation of what
considerations are needed (very few).
---------
Co-authored-by: Stephen Tozer <Melamoto@gmail.com>
I was studying the code here and realized that the comments were talking
about grouping by basic blocks when the code was grouping by Function.
Fix the comments so they reflect what the code is actually doing.
Symbols that get mapped into the read-only section are loaded as part of
the text segment and will always need a TOC entry to be addressable. Add
an option to aggressively merge these read only globals to reduce TOC
usage.
As a side-effect of PR #101222, GlobalMerge started making transforms
which are unsafe on Mach-O platforms.
Two issues, in particular, are fixed here:
1. We must never merge symbols in the `__cfstring` section, as the
linker assumes each object in this section is only ever referenced
directly, and that it can split the section as it likes.
Previously, we avoided this problem because CFString literals are
identified by private-linkage symbols. This patch adds a list of
section-names with special behavior, to avoid merging under Mach-O.
2. When GlobalMerge code was originally written, it had to be careful
about emitting symbol aliases, due to issues with Mach-O's subsection
splitting in the linker with `-dead_strip` enabled. The underlying cause
of this problem was fixed in 2016, via creation of the `.alt_entry`
assembler directive, which allows a symbol to not also imply the start
of a new subsection. GlobalMerge's workaround for that issue was never
removed.
In the meantime, Apple's new ld-prime linker was written, and has a bug
in `.alt_entry` handling. Therefore, even though the original issue was
fixed, we must _continue_ to be careful not to emit any such symbol
aliases. The existing workaround avoided it for InternalLinkage symbols,
but after the above-mentioned PR, we also must avoid emitting aliases
for PrivateLinkage symbols.
I will file an Apple bug-report about this issue, so that it can be
fixed in a future version of ld-prime. But, in the meantime, the
workaround is sufficient for GlobalMerge, unless
`-global-merge-on-externals` is enabled (which it is already not by
default, on MachO platforms, due to the original issue).
Fixes#104625
This patch aims to reduce TOC usage by merging internal and private
global data.
Moreover, we also add the GlobalMerge pass within the PPCTargetMachine
pipeline, which is disabled by default. This transformation can be
enabled by -ppc-global-merge.
This patch updates the GlobalMerge pass to be able to handle private
global variables, which is required for a follow-up PowerPC specific
GlobalMerge patch to merge internal and private globals.
A new LIT test is also added to exhibit the ability to merge private
globals.
DenseMap iteration order is not guaranteed to be deterministic.
Without the change,
llvm/test/Transforms/GlobalMerge/basic.ll could fail when
`combineHashValue` changes (#95970).
We add a feature that prevents the GlobalMerge pass from considering
data smaller than a minimum size in bytes for merging.
The MinSize is set in 3 ways:
1. If global-merge-min-data-size is explicitly set, then it uses that
value.
2. If SmallDataLimit is set and non-zero, then SmallDataLimit + 1 is
used.
3. Otherwise, 0 is used, which means all sizes are considered for
merging.
We found that this feature allowed us to see the benefit of the
GlobalMerge pass while eliminating some merging that was not beneficial.
This feature allowed us to enable the GlobalMerge pass on RISC-V in our
downstream by default because it led to improvements on multiple
benchmark suites.
I plan to post a separate patch to propose enabling this by default on
RISC-V. But I do not want that discussion to be part of the discussion
of adding this feature, so I am keeping the patches separate.
Noticed while reviewing the code.
If the resize causes a new allocation, this will fill the new allocation
with zeroes directly. Previously, we would fill the old allocation with
zeroes, then copy them to the new allocation before filling the
additional space with zeros.
This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::{starts,ends}_with in C++20.
I'm planning to deprecate and eventually remove
StringRef::{starts,ends}with.
Adds an IR pass for -fsanitize=memtag-globals. This pass goes over the
tag-capable global variables, and replaces them with a tagged global
variable of the same contents. This new global variable will have its
size and alignment adjusted if neccesary so that they're both a multiple
of the tag granule size (16 bytes).
Global merge must also be suppressed for tagged globals, as each global
variable must have a unique tag. This can possibly be relaxed in future;
globals that are identical in size, alignment, and content can possibly
be merged. The major problem comes from tail- or head-merging, which if
left unchecked, could have partially-overlapping global variables with
different memory tags, leading to crashes at runtime.
Reviewed By: fmayer, eugenis
Differential Revision: https://reviews.llvm.org/D133392
This reverts commit 4edfcff71e150770675a19576f698c7bbe788ee2.
Broke the non-aarch64-containing target builds.
https://reviews.llvm.org/D133392 has more context.
Adds an IR pass for -fsanitize=memtag-globals. This pass goes over the
tag-capable global variables, and replaces them with a tagged global
variable of the same contents. This new global variable will have its
size and alignment adjusted if neccesary so that they're both a multiple
of the tag granule size (16 bytes).
Global merge must also be suppressed for tagged globals, as each global
variable must have a unique tag. This can possibly be relaxed in future;
globals that are identical in size, alignment, and content can possibly
be merged. The major problem comes from tail- or head-merging, which if
left unchecked, could have partially-overlapping global variables with
different memory tags, leading to crashes at runtime.
Reviewed By: fmayer, eugenis
Differential Revision: https://reviews.llvm.org/D133392
The filter clause in the landingpad may not have a GlobalVariable operand.
It may instead have a ConstantArray of operands and each operand within this
ConstantArray should also be checked to see if it is a GlobalVariable.
This patch add the check for the ConstantArray as well as a debug message that
outputs the contents of MustKeepGlobalVariables.
Reviewed By: lei, amyk, scui
Differential Revision: https://reviews.llvm.org/D128287
We don't support global variables with scalable vector types so I've
changed the code to compare the fixed sizes instead.
Differential Revision: https://reviews.llvm.org/D88564
Replace with forward declarations and move includes down to source files where required.
I also needed to move the TargetLoweringObjectFile::SectionForGlobal wrapper implementation down into TargetLoweringObjectFile.cpp
This is how it should've been and brings it more in line with
std::string_view. There should be no functional change here.
This is mostly mechanical from a custom clang-tidy check, with a lot of
manual fixups. It uncovers a lot of minor inefficiencies.
This doesn't actually modify StringRef yet, I'll do that in a follow-up.
Symbols created for merged external global variables have default
visibility. This can break programs when compiling with -Oz
-fvisibility=hidden as symbols that should be hidden will be exported at
link time.
Differential Revision: https://reviews.llvm.org/D73235
This file lists every pass in LLVM, and is included by Pass.h, which is
very popular. Every time we add, remove, or rename a pass in LLVM, it
caused lots of recompilation.
I found this fact by looking at this table, which is sorted by the
number of times a file was changed over the last 100,000 git commits
multiplied by the number of object files that depend on it in the
current checkout:
recompiles touches affected_files header
342380 95 3604 llvm/include/llvm/ADT/STLExtras.h
314730 234 1345 llvm/include/llvm/InitializePasses.h
307036 118 2602 llvm/include/llvm/ADT/APInt.h
213049 59 3611 llvm/include/llvm/Support/MathExtras.h
170422 47 3626 llvm/include/llvm/Support/Compiler.h
162225 45 3605 llvm/include/llvm/ADT/Optional.h
158319 63 2513 llvm/include/llvm/ADT/Triple.h
140322 39 3598 llvm/include/llvm/ADT/StringRef.h
137647 59 2333 llvm/include/llvm/Support/Error.h
131619 73 1803 llvm/include/llvm/Support/FileSystem.h
Before this change, touching InitializePasses.h would cause 1345 files
to recompile. After this change, touching it only causes 550 compiles in
an incremental rebuild.
Reviewers: bkramer, asbirlea, bollu, jdoerfert
Differential Revision: https://reviews.llvm.org/D70211
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
Summary:
Global variables that are external and zero initialized are
supposed to be merged with global variables in the bss section
rather than the data section.
Reviewers: efriedma, rengolin, t.p.northover, javed.absar, asl, john.brawn, pcc
Reviewed By: efriedma
Subscribers: dmgreen, llvm-commits
Differential Revision: https://reviews.llvm.org/D51379
llvm-svn: 341008
This should more accurately reflect what the AsmPrinter will actually
do.
This is NFC, as far as I can tell; all the places that might be affected
already have an extra check to avoid using the result of
getPreferredAlignment in this situation.
Differential Revision: https://reviews.llvm.org/D51377
llvm-svn: 340999
At least on ELF, it's impossible to tell from the object file whether
two globals with the same section marking were merged: the merged global
uses "private" linkage to hide its symbol, and the aliases look like
regular symbols. I can't think of any other reason to disallow it.
(Of course, we can only merge globals in the same section.)
The weird alignment handling matches AsmPrinter; our alignment handling
for global variables should probably be refactored.
Differential Revision: https://reviews.llvm.org/D49822
llvm-svn: 338791
Reuse the handling for llvm.used, and don't transform such globals.
Fixes a failure on the asan buildbot caused by my previous commit.
llvm-svn: 337973
Instead of depending on implicit padding from the structure layout code,
use a packed struct and emit the padding explicitly.
Differential Revision: https://reviews.llvm.org/D49710
llvm-svn: 337961
If no alignment is set, the abi/preferred alignment of structs will be
used which may be higher than required. This can lead to extra padding
and in the end an increase in data size.
Differential Revision: https://reviews.llvm.org/D47633
llvm-svn: 334099
