Unlike SDag, we use a separate G_GEP instruction (much simplified, only taking
a single byte offset) to preserve the pointer type information through
selection.
llvm-svn: 281205
Remove createNode() and any API that depending on it, and add
HasCreateNode to the list of checks for HasObsoleteCustomizations. Now
an ilist *never* allocates (this was already true for iplist).
This factors out all the differences between iplist and ilist. I'll aim
to rename both to "owning_ilist" eventually, to call out the interesting
(not exactly intrusive) ownership semantics. In the meantime, I've left
both names around to reduce code churn.
One of the deleted APIs is the ilist copy constructor. I've lifted up
and tested iplist::cloneFrom (ala simple_ilist::cloneFrom) as a
replacement.
Users of ilist<> and iplist<> that want the list to allocate nodes have
a few options:
- use std::list;
- use AllocatorList or BumpPtrList (or build a similarly trivial list);
- use cloneFrom (which is explicit at the call site); or
- allocate at the call site.
See r280573, r281177, r281181, and r281182 for examples of what to do if
you're updating out-of-tree code.
llvm-svn: 281184
- Add AllocatorList, a non-intrusive list that owns an LLVM-style
allocator and provides a std::list-like interface (trivially built on
top of simple_ilist),
- add a typedef (and unit tests) for BumpPtrList, and
- use BumpPtrList for the list of llvm::yaml::Token (i.e., TokenQueueT).
TokenQueueT has no need for the complexity of an intrusive list. The
only reason to inherit from ilist was to customize the allocator.
TokenQueueT was the only example in-tree of using ilist<> in a truly
non-intrusive way.
Moreover, this removes the final use of the non-intrusive
ilist_traits<>::createNode (after r280573, r281177, and r281181). I
have a WIP patch that removes this customization point (and the API that
relies on it) that I plan to commit soon.
Note: AllocatorList owns the allocator, which limits the viable API
(e.g., splicing must be on the same list). For now I've left out
any problematic API. It wouldn't be hard to split AllocatorList into
two layers: an Impl class that calls DerivedT::getAlloc (via CRTP), and
derived classes that handle Allocator ownership/reference/etc semantics;
and then implement splice with appropriate assertions; but TBH we should
probably just customize the std::list allocators at that point.
llvm-svn: 281182
Force IVUsers to be moved instead of copied, properly update Parent
pointers in IVStrideUse when IVUsers is moved, and make sure we have
move constructors available in iplist and ilist.
I came across this in a WIP patch that deleted the copy constructors
from ilist. I was surprised to find that IVUsersAnalysis couldn't be
registered in the new pass manager.
It's not clear to me whether IVUsers was getting moved only when empty,
but if it was being moved when it was non-empty then this fixes a
pointer invalidation bug and should give some sort of speedup. Note
that the bugfix would be necessary even for a copy constructor.
llvm-svn: 281181
ilist_iterator::reset was unnecessary API, and wasn't any clearer (or
safer) at the call site than constructing a temporary and assigning it
to the iterator.
llvm-svn: 281175
Now that MachineBasicBlock::reverse_instr_iterator knows when it's at
the end (since r281168 and r281170), implement
MachineBasicBlock::reverse_iterator directly on top of an
ilist::reverse_iterator by adding an IsReverse template parameter to
MachineInstrBundleIterator. This replaces another hard-to-reason-about
use of std::reverse_iterator on list iterators, matching the changes for
ilist::reverse_iterator from r280032 (see the "out of scope" section at
the end of that commit message). MachineBasicBlock::reverse_iterator
now has a handle to the current node and has obvious invalidation
semantics.
r280032 has a more detailed explanation of how list-style reverse
iterators (invalidated when the pointed-at node is deleted) are
different from vector-style reverse iterators like std::reverse_iterator
(invalidated on every operation). A great motivating example is this
commit's changes to lib/CodeGen/DeadMachineInstructionElim.cpp.
Note: If your out-of-tree backend deletes instructions while iterating
on a MachineBasicBlock::reverse_iterator or converts between
MachineBasicBlock::iterator and MachineBasicBlock::reverse_iterator,
you'll need to update your code in similar ways to r280032. The
following table might help:
[Old] ==> [New]
delete &*RI, RE = end() delete &*RI++
RI->erase(), RE = end() RI++->erase()
reverse_iterator(I) std::prev(I).getReverse()
reverse_iterator(I) ++I.getReverse()
--reverse_iterator(I) I.getReverse()
reverse_iterator(std::next(I)) I.getReverse()
RI.base() std::prev(RI).getReverse()
RI.base() ++RI.getReverse()
--RI.base() RI.getReverse()
std::next(RI).base() RI.getReverse()
(For more details, have a look at r280032.)
llvm-svn: 281172
Add an assertion to the MachineInstrBundleIterator from instr_iterator
that the underlying iterator is valid. This is possible know that we
can check ilist_node::isSentinel (since r281168), and is consistent with
the constructors from MachineInstr* and MachineInstr&.
Avoiding the new assertion in operator== and operator!= requires four
(!!!!) new overloads each.
(As an aside, I'm strongly in favour of:
- making the conversion from instr_iterator explicit;
- making the conversion from pointer explicit;
- making the conversion from reference explicit; and
- removing all the extra overloads of operator== and operator!= except
const_instr_iterator.
I'm not signing up for that at this point, but being clear about when
something is an MachineInstr-iterator (possibly instr_end()) vs
MachineInstr-bundle-iterator (possibly end()) vs MachineInstr* (possibly
nullptr) vs MachineInstr& (known valid) would surely make code
cleaner... and it would remove a ton of boilerplate from
MachineInstrBundleIterator operators.)
llvm-svn: 281170
This is a prep commit before fixing MachineBasicBlock::reverse_iterator
invalidation semantics, ala r281167 for ilist::reverse_iterator. This
changes MachineBasicBlock::Instructions to track which node is the
sentinel regardless of LLVM_ENABLE_ABI_BREAKING_CHECKS.
There's almost no functionality change (aside from ABI). However, in
the rare configuration:
#if !defined(NDEBUG) && !defined(LLVM_ENABLE_ABI_BREAKING_CHECKS)
the isKnownSentinel() assertions in ilist_iterator<>::operator* suddenly
have teeth for MachineInstr. If these assertions start firing for your
out-of-tree backend, have a look at the suggestions in the commit
message for r279314, and at some of the commits leading up to it that
avoid dereferencing the end() iterator.
llvm-svn: 281168
This adds two declarative configuration options for intrusive lists
(available for simple_ilist, iplist, and ilist). Both of these options
affect ilist_node interoperability and need to be passed both to the
node and the list. Instead of adding a new traits class, they're
specified as optional template parameters (in any order).
The two options:
1. Pass ilist_sentinel_tracking<true> or ilist_sentinel_tracking<false>
to control whether there's a bit on ilist_node "prev" pointer
indicating whether it's the sentinel. The default behaviour is to
use a bit if and only if LLVM_ENABLE_ABI_BREAKING_CHECKS.
2. Pass ilist_tag<TagA> and ilist_tag<TagB> to allow insertion of a
single node into two different lists (simultaneously).
I have an immediate use-case for (1) ilist_sentinel_tracking: fixing the
validation semantics of MachineBasicBlock::reverse_iterator to match
ilist::reverse_iterator (ala r280032: see the comments at the end of the
commit message there). I'm adding (2) ilist_tag in the same commit to
validate that the options framework supports expansion. Justin Bogner
mentioned this might enable a possible cleanup in SelectionDAG, but I'll
leave this to others to explore. In the meantime, the unit tests and
the comments for simple_ilist and ilist_node have usage examples.
Note that there's a layer of indirection to support optional,
out-of-order, template paramaters. Internal classes are templated on an
instantiation of the non-variadic ilist_detail::node_options.
User-facing classes use ilist_detail::compute_node_options to compute
the correct instantiation of ilist_detail::node_options.
The comments for ilist_detail::is_valid_option describe how to add new
options (e.g., ilist_packed_int<int NumBits>).
llvm-svn: 281167
Summary:
An IR load can be invariant, dereferenceable, neither, or both. But
currently, MI's notion of invariance is IR-invariant &&
IR-dereferenceable.
This patch splits up the notions of invariance and dereferenceability at
the MI level. It's NFC, so adds some probably-unnecessary
"is-dereferenceable" checks, which we can remove later if desired.
Reviewers: chandlerc, tstellarAMD
Subscribers: jholewinski, arsenm, nemanjai, llvm-commits
Differential Revision: https://reviews.llvm.org/D23371
llvm-svn: 281151
should have been (1ULL << MCID::XYZ). Currently this works because enum Flag
has 31 values, but extending it will result in a compile warnings/errors.
This was part of the accepted patch in https://reviews.llvm.org/D23601, but
it was suggested to apply this first as a separate patch.
llvm-svn: 281149
... and make a few ilist-internal API changes, in preparation for
changing how ilist_node is templated. The only effect for ilist users
should be changing the friend target from llvm::ilist_node_access to
llvm::ilist_detail::NodeAccess (which is only necessary when they
inherit privately from ilist_node).
- Split out SpecificNodeAccess, which has overloads of getNodePtr and
getValuePtr that are untemplated.
- Use more typedefs to prevent more changes later.
- Force inheritance to use *NodeAccess (to emphasize that ilist *users*
shouldn't be doing this).
There should be no functionality change here.
llvm-svn: 281142
Summary:
I want to separate out the notions of invariance and dereferenceability
at the MI level, so that they correspond to the equivalent concepts at
the IR level. (Currently an MI load is MI-invariant iff it's
IR-invariant and IR-dereferenceable.)
First step is renaming this function.
Reviewers: chandlerc
Subscribers: MatzeB, jfb, llvm-commits
Differential Revision: https://reviews.llvm.org/D23370
llvm-svn: 281125
I tested this with "ninja check-llvm-codegen" on a Release build with
all architectures enabled, and again with a Debug build on x86.
Found with llvm-cov.
Differential Revision: https://reviews.llvm.org/D24433
llvm-svn: 281120
SmallVectors are convenient, but they don't cover every use case.
In particular, they are fairly large (3 pointers + one element) and
there is no way to take ownership of the buffer to put it somewhere
else. This patch then adds a lower lever interface that works with
any buffer.
llvm-svn: 281082
This simplifies a lot of code, and will actually be necessary for
an upcoming patch to serialize TPI record hash values.
The idea before was that visitors should be examining records, not
modifying them. But this is no longer true with a visitor that
constructs a CVRecord from Yaml. To handle this until now, we
were doing some fixups on CVRecord objects at a higher level, but
the code is really awkward, and it makes sense to just have the
visitor write the bytes into the CVRecord. In doing so I uncovered
a few bugs related to `Data` and `RawData` and fixed those.
Reviewed By: rnk
Differential Revision: https://reviews.llvm.org/D24362
llvm-svn: 281067
This writes the full sequence of type records described in
Yaml to the TPI stream of the PDB file.
Reviewed By: rnk
Differential Revision: https://reviews.llvm.org/D24316
llvm-svn: 281063
These were added in r281051, which, I am embarrassed to admit, has an
incomplete commit message that I forgot to update before pushing. You
can ignore element (2) in that list.
llvm-svn: 281054
1) On some platforms, sizeof(SDNodeBits) == 1, so we were only copying
one byte out of the bitfield when we wanted to copy two, and we were
leaving half of the return value of getRawSubclassData() undefined.
2) Something something bitfields, not sure exactly what the issue or fix
is, yet. (TODO)
Summary:
Previously we were assuming that SDNodeBits covered all of SDNode's
anonymous subclass data bitfield union. But that's not right; it might
have size 1, in which it clearly doesn't.
This patch adds a field that does cover the whole union and adds
static_asserts to ensure it stays correct.
Reviewers: ahatanak, chandlerc
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D24223
llvm-svn: 281051
These instructions were only necessary when type information was stored in the
MachineInstr (because only generic MachineInstrs possessed a type). Now that
it's in MachineRegisterInfo, COPY and PHI work fine.
llvm-svn: 281037
We want each register to have a canonical type, which means the best place to
store this is in MachineRegisterInfo rather than on every MachineInstr that
happens to use or define that register.
Most changes following from this are pretty simple (you need an MRI anyway if
you're going to be doing any transformations, so just check the type there).
But legalization doesn't really want to check redundant operands (when, for
example, a G_ADD only ever has one type) so I've made use of MCInstrDesc's
operand type field to encode these constraints and limit legalization's work.
As an added bonus, more validation is possible, both in MachineVerifier and
MachineIRBuilder (coming soon).
llvm-svn: 281035
Summary:
While woring on mapping attributes in the C API, it clearly appeared that the recent changes in the API on the C++ side left Function and Call/Invoke with an attribute API that grew in an ad hoc manner. This makes it difficult to work with it, because one doesn't know which overloads exists and which do not.
Make sure that getter/setter function exists for both enum and string version. Remove inconsistent getter/setter, unless they have many callsites.
This should make it easier to work with attributes in the future.
This doesn't change how attribute works.
Reviewers: bkramer, whitequark, mehdi_amini, void
Subscribers: llvm-commits
Differential Revision: http://reviews.llvm.org/D21514
llvm-svn: 281019
mapping a yaml field to an object in code has always been
a stateless operation. You could still pass state by using the
`setContext` function of the YAMLIO object, but this represented
global state for the entire yaml input. In order to have
context-sensitive state, it is necessary to pass this state in
at the granularity of an individual mapping.
This patch adds support for this type of context-sensitive state.
You simply pass an additional argument of type T to the
`mapRequired` or `mapOptional` functions, and provided you have
specialized a `MappingContextTraits<U, T>` class with the
appropriate mapping function, you can pass this context into
the mapping function.
Reviewed By: chandlerc
Differential Revision: https://reviews.llvm.org/D24162
llvm-svn: 280977
And associated commits, as they broke the Thumb bots.
This reverts commit r280935.
This reverts commit r280891.
This reverts commit r280888.
llvm-svn: 280967
Summary:
This allows specifying instructions that are available only in specific assembler variant. If AsmVariantName is specified then instruction will be presented only in MatchTable for this variant. If not specified then assembler variants will be determined based on AsmString.
Also this allows splitting assembler match tables in same way as it is done in dissasembler.
Reviewers: ab, tstellarAMD, craig.topper, vpykhtin
Subscribers: wdng
Differential Revision: https://reviews.llvm.org/D24249
llvm-svn: 280952
Refactor replaceDominatedUsesWith to have a flag to control whether to replace uses in BB itself.
Summary: This is in preparation for LoopSink pass which calls replaceDominatedUsesWith to update after sinking.
llvm-svn: 280949
CGP tail-duplicates rets into blocks that end with a call that feed the ret.
This puts the call in tail position, potentially allowing the DAG builder to
lower it as a tail call. To avoid tail duplication in cases where we won't
form the tail call, CGP tried to predict whether this is going to be possible,
and avoids doing it when lowering as a tail call will definitely fail.
However, it was being too conservative by always throwing away calls to
functions with a signext/zeroext attribute on the return type.
Instead, we can use the same logic the builder uses to determine whether the
attributes work out.
Differential Revision: https://reviews.llvm.org/D24315
llvm-svn: 280894
This is a port of XRay to ARM 32-bit, without Thumb support yet. The XRay instrumentation support is moving up to AsmPrinter.
This is one of 3 commits to different repositories of XRay ARM port. The other 2 are:
1. https://reviews.llvm.org/D23932 (Clang test)
2. https://reviews.llvm.org/D23933 (compiler-rt)
Differential Revision: https://reviews.llvm.org/D23931
llvm-svn: 280888
When branching to a block that immediately tail calls, it is possible to fold
the call directly into the branch if the call is direct and there is no stack
adjustment, saving one byte.
Example:
define void @f(i32 %x, i32 %y) {
entry:
%p = icmp eq i32 %x, %y
br i1 %p, label %bb1, label %bb2
bb1:
tail call void @foo()
ret void
bb2:
tail call void @bar()
ret void
}
before:
f:
movl 4(%esp), %eax
cmpl 8(%esp), %eax
jne .LBB0_2
jmp foo
.LBB0_2:
jmp bar
after:
f:
movl 4(%esp), %eax
cmpl 8(%esp), %eax
jne bar
.LBB0_1:
jmp foo
I don't expect any significant size savings from this (on a Clang bootstrap I
saw 288 bytes), but it does make the code a little tighter.
This patch only does 32-bit, but 64-bit would work similarly.
Differential Revision: https://reviews.llvm.org/D24108
llvm-svn: 280832
Summary:
Previously we were trying to represent this with the "contains" list of
the .cv_inline_linetable directive, which was not enough information.
Now we directly represent the chain of inlined call sites, so we know
what location to emit when we encounter a .cv_loc directive of an inner
inlined call site while emitting the line table of an outer function or
inlined call site. Fixes PR29146.
Also fixes PR29147, where we would crash when .cv_loc directives crossed
sections. Now we write down the section of the first .cv_loc directive,
and emit an error if any other .cv_loc directive for that function is in
a different section.
Also fixes issues with discontiguous inlined source locations, like in
this example:
volatile int unlikely_cond = 0;
extern void __declspec(noreturn) abort();
__forceinline void f() {
if (!unlikely_cond) abort();
}
int main() {
unlikely_cond = 0;
f();
unlikely_cond = 0;
}
Previously our tables gave bad location information for the 'abort'
call, and the debugger wouldn't snow the inlined stack frame for 'f'.
It is important to emit good line tables for this code pattern, because
it comes up whenever an asan bug occurs in an inlined function. The
__asan_report* stubs are generally placed after the normal function
epilogue, leading to discontiguous regions of inlined code.
Reviewers: majnemer, amccarth
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D24014
llvm-svn: 280822
This adds a copy of the demangler in libcxxabi.
The code also has no dependencies on anything else in LLVM. To enforce
that I added it as another library. That way a BUILD_SHARED_LIBS will
fail if anyone adds an use of StringRef for example.
The no llvm dependency combined with the fact that this has to build
on linux, OS X and Windows required a few changes to the code. In
particular:
No constexpr.
No alignas
On OS X at least this library has only one global symbol:
__ZN4llvm16itanium_demangleEPKcPcPmPi
My current plan is:
Commit something like this
Change lld to use it
Change lldb to use it as the fallback
Add a few #ifdefs so that exactly the same file can be used in
libcxxabi to export abi::__cxa_demangle.
Once the fast demangler in lldb can handle any names this
implementation can be replaced with it and we will have the one true
demangler.
llvm-svn: 280732
Currently the pass updates branch weights in the IR if the function has
any PGO info (entry frequency is set). However we could still have
regions of the CFG that does not have branch weights collected (e.g. a
cold region). In this case we'd use static estimates. Since static
estimates for branches are determined independently, they are
inconsistent. Updating them can "randomly" inflate block frequencies.
I've run into this in a completely cold loop of h264ref from
SPEC. -Rpass-with-hotness showed the loop to be completely cold during
inlining (before JT) but completely hot during vectorization (after JT).
The new testcase demonstrate the problem. We check array elements
against 1, 2 and 3 in a loop. The check against 3 is the loop-exiting
check. The block names should be self-explanatory.
In this example, jump threading incorrectly updates the weight of the
loop-exiting branch to 0, drastically inflating the frequency of the
loop (in the range of billions).
There is no run-time profile info for edges inside the loop, so branch
probabilities are estimated. These are the resulting branch and block
frequencies for the loop body:
check_1 (16)
(8) / |
eq_1 | (8)
\ |
check_2 (16)
(8) / |
eq_2 | (8)
\ |
check_3 (16)
(1) / |
(loop exit) | (15)
|
(back edge)
First we thread eq_1 -> check_2 to check_3. Frequencies are updated to
remove the frequency of eq_1 from check_2 and then from the false edge
leaving check_2. Changed frequencies are highlighted with * *:
check_1 (16)
(8) / |
eq_1~ | (8)
/ |
/ check_2 (*8*)
/ (8) / |
\ eq_2 | (*0*)
\ \ |
` --- check_3 (16)
(1) / |
(loop exit) | (15)
|
(back edge)
Next we thread eq_1 -> check_3 and eq_2 -> check_3 to check_1 as new
back edges. Frequencies are updated to remove the frequency of eq_1 and
eq_3 from check_3 and then the false edge leaving check_3 (changed
frequencies are highlighted with * *):
check_1 (16)
(8) / |
eq_1~ | (8)
/ |
/ check_2 (*8*)
/ (8) / |
/-- eq_2~ | (*0*)
(back edge) |
check_3 (*0*)
(*0*) / |
(loop exit) | (*0*)
|
(back edge)
As a result, the loop exit edge ends up with 0 frequency which in turn makes
the loop header to have maximum frequency.
There are a few potential problems here:
1. The profile data seems odd. There is a single profile sample of the
loop being entered. On the other hand, there are no weights inside the
loop.
2. Based on static estimation we shouldn't set edges to "extreme"
values, i.e. extremely likely or unlikely.
3. We shouldn't create profile metadata that is calculated from static
estimation. I am not sure what policy is but it seems to make sense to
treat profile metadata as something that is known to originate from
profiling. Estimated probabilities should only be reflected in BPI/BFI.
Any one of these would probably fix the immediate problem. I went for 3
because I think it's a good policy to have and added a FIXME about 2.
Differential Revision: https://reviews.llvm.org/D24118
llvm-svn: 280713
Use ADT/BitmaskEnum for DINode::DIFlags for the following purposes:
Get rid of unsigned int for flags to avoid problems on platforms with sizeof(int) < 4
Flags are now strongly typed
Patch by: Victor Leschuk <vleschuk@gmail.com>
Differential Revision: https://reviews.llvm.org/D23766
llvm-svn: 280700
