Lama Saba [Thu, 18 May 2017 08:11:50 +0000 (08:11 +0000)]
[X86] Replace slow LEA instructions in X86
According to Intel's Optimization Reference Manual for SNB+:
" For LEA instructions with three source operands and some specific situations, instruction latency has increased to 3 cycles, and must
dispatch via port 1:
- LEA that has all three source operands: base, index, and offset
- LEA that uses base and index registers where the base is EBP, RBP,or R13
- LEA that uses RIP relative addressing mode
- LEA that uses 16-bit addressing mode "
This patch currently handles the first 2 cases only.
George Rimar [Thu, 18 May 2017 08:00:01 +0000 (08:00 +0000)]
[lib/Object] - Minor API update for llvm::Decompressor.
I revisited Decompressor API (issue with it was triggered during D32865 review)
and found it is probably provides more then we really need.
Issue was about next method's signature:
Error decompress(SmallString<32> &Out);
It is too strict. At first I wanted to change it to decompress(SmallVectorImpl<char> &Out),
but then found it is still not flexible because sticks to SmallVector.
During reviews was suggested to use templating to simplify code. Patch do that.
Serguei Katkov [Thu, 18 May 2017 06:11:56 +0000 (06:11 +0000)]
[BPI] Reduce the probability of unreachable edge to minimal value greater than 0
The probability of edge coming to unreachable block should be as low as possible.
The change reduces the probability to minimal value greater than zero.
The bug https://bugs.llvm.org/show_bug.cgi?id=32214 show the example when
the probability of edge coming to unreachable block is greater than for edge
coming to out of the loop and it causes incorrect loop rotation.
Please note that with this change the behavior of unreachable heuristic is a bit different
than others. Specifically, before this change the sum of probabilities
coming to unreachable blocks have the same weight for all branches
(it was just split over all edges of this block coming to unreachable blocks).
With this change it might be slightly different but not to much due to probability of
taken branch to unreachable block is really small.
Akira Hatanaka [Thu, 18 May 2017 03:52:29 +0000 (03:52 +0000)]
[ThinLTO] Do not assert when adding a module with a different but
compatible target triple
Currently, an assertion fails in ThinLTOCodeGenerator::addModule when
the target triple of the module being added doesn't match that of the
one stored in TMBuilder. This patch relaxes the constraint and makes
changes to allow target triples that only differ in their version
numbers on Apple platforms, similarly to what r228999 did.
Craig Topper [Thu, 18 May 2017 00:51:39 +0000 (00:51 +0000)]
[Statistics] Add a method to atomically update a statistic that contains a maximum
Summary:
There are several places in the codebase that try to calculate a maximum value in a Statistic object. We currently do this in one of two ways:
The first version reads from MaxNumFoo one time and uncontionally rwrites to it. The second version possibly reads it twice depending on the result of the first compare. But we have no way of knowing if the value was changed by another thread between the reads and the writes.
This patch adds a method to the Statistic object that can ensure that we only store if our value is the max and the previous max didn't change after we read it. If it changed we'll recheck if our value should still be the max or not and try again.
This spawned from an audit I'm trying to do of all places we uses the implicit conversion to unsigned on the Statistics objects. See my previous thread on llvm-dev https://groups.google.com/forum/#!topic/llvm-dev/yfvxiorKrDQ
Craig Topper [Wed, 17 May 2017 23:22:10 +0000 (23:22 +0000)]
[Statistics] Use Statistic::operator+= instead of adding and assigning separately.
I believe this technically fixes a multithreaded race condition in this code. But my primary concern was as part of looking at removing the ability to treat Statistics like a plain unsigned. There are many weird operations on Statistics in the codebase.
This breaks the GISel bot:
http://lab.llvm.org:8080/green/job/Compiler_Verifiers_GlobalISEL/5163/consoleFull#-134276167849ba4694-19c4-4d7e-bec5-911270d8a58c
Sanjay Patel [Wed, 17 May 2017 22:29:40 +0000 (22:29 +0000)]
[InstCombine] handle icmp i1 X, C early to avoid creating an unknown pattern
The missing optimization for xor-of-icmps still needs to be added, but by
being more efficient (not generating unnecessary logic ops with constants)
we avoid the bug.
See discussion in post-commit comments:
https://reviews.llvm.org/D32143
Sanjay Patel [Wed, 17 May 2017 22:15:07 +0000 (22:15 +0000)]
[InstCombine] move icmp bool canonicalizations to helper; NFC
As noted in the post-commit comments in D32143, we should be
catching the constant operand cases sooner to be more efficient
and less likely to expose a missing fold.
Matt Arsenault [Wed, 17 May 2017 21:56:25 +0000 (21:56 +0000)]
AMDGPU: Start defining a calling convention
Partially implement callee-side for arguments and return values.
byval doesn't work properly, and most likely sret or other on-stack
return values most as well.
Kyle Butt [Wed, 17 May 2017 21:54:41 +0000 (21:54 +0000)]
CodeGen: Power: Add lowering for shifts of v1i128.
When legalizing vector operations on vNi128, they will be split to v1i128
because that is a legal type on ppc64, but then the compiler will crash in
selection dag because it fails to select for these operations. This patch fixes
shift operations. Logical shift right and left shift can be performed in the
vector unit, but algebraic shift right requires being split.
Michael Liao [Wed, 17 May 2017 21:48:00 +0000 (21:48 +0000)]
Fix PR33028
- '-verify-mahcineinstrs' starts to complain allocatable live-in physical
registers on non-entry or non-landing-pad basic blocks.
- Refactor the XBEGIN translation to define EAX on a dedicated fallback code
path due to XABORT. Add a pseudo instruction to define EAX explicitly to
avoid add physical register live-in.
Matt Arsenault [Wed, 17 May 2017 21:23:14 +0000 (21:23 +0000)]
AMDGPU: Expand frame indexes to be relative to scratch wave offset
In order for an arbitrary callee to access an object
in a caller's stack frame, the 32-bit offset used as
the private pointer needs to be relative to the kernel's
scratch wave offset register.
Convert to this by finding the difference from the current
stack frame and scaling by the wavefront size.
Bob Haarman [Wed, 17 May 2017 20:46:48 +0000 (20:46 +0000)]
[llvm-pdbdump] in yaml2pdb, generate default output filename if none given
Summary:
llvm-pdbdump yaml2pdb used to fail with a misleading error
message ("An I/O error occurred on the file system") if no output file
was specified. This change adds an assert to PDBFileBuilder to check
that an output file name is specified, and makes llvm-pdbdump generate
an output file name based on the input file name if no output file
name is explicitly specified.
Sanjay Patel [Wed, 17 May 2017 20:27:55 +0000 (20:27 +0000)]
[InstSimplify] handle all icmp i1 X, C in one place; NFCI
We already handled all of the new tests identically, but several
of those went through a lot of unnecessary processing before
getting folded.
Another motivation for grouping these cases together is that
InstCombine needs a similar fold. Currently, it handles the
'not' cases inefficiently which can lead to bugs as described
in the post-commit comments of:
https://reviews.llvm.org/D32143
Zachary Turner [Wed, 17 May 2017 20:23:31 +0000 (20:23 +0000)]
[BinaryStream] Reduce the amount of boiler plate needed to use.
Often you have an array and you just want to use it. With the current
design, you have to first construct a `BinaryByteStream`, and then create
a `BinaryStreamRef` from it. Worse, the `BinaryStreamRef` holds a pointer
to the `BinaryByteStream`, so you can't just create a temporary one to
appease the compiler, you have to actually hold onto both the `ArrayRef`
as well as the `BinaryByteStream` *AND* the `BinaryStreamReader` on top of
that. This makes for very cumbersome code, often requiring one to store a
`BinaryByteStream` in a class just to circumvent this.
At the cost of some added complexity (not exposed to users, but internal
to the library), we can do better than this. This patch allows us to
construct `BinaryStreamReaders` and `BinaryStreamWriters` directly from
source data (e.g. `StringRef`, `MutableArrayRef<uint8_t>`, etc). Not only
does this reduce the amount of code you have to type and make it more
obvious how to use it, but it solves real lifetime issues when it's
inconvenient to hold onto a `BinaryByteStream` for a long time.
The additional complexity is in the form of an added layer of indirection.
Whereas before we simply stored a `BinaryStream*` in the ref, we now store
both a `BinaryStream*` **and** a `std::shared_ptr<BinaryStream>`. When
the user wants to construct a `BinaryStreamRef` directly from an
`ArrayRef` etc, we allocate an internal object that holds ownership over a
`BinaryByteStream` and forwards all calls, and store this in the
`shared_ptr<>`. This also maintains the ref semantics, as you can copy it
by value and references refer to the same underlying stream -- the one
being held in the object stored in the `shared_ptr`.
Dimitry Andric [Wed, 17 May 2017 19:46:49 +0000 (19:46 +0000)]
Reapply part of rL303015, fixing just the DynamicLibaryTest. Add
retrieval of the original argv[0] from the GoogleTest framework, so it
is more likely the correct main executable path is found.
Dimitry Andric [Wed, 17 May 2017 19:33:10 +0000 (19:33 +0000)]
Revert r303015, because it has the unintended side effect of breaking
driver-mode recognition in clang (this is because the sysctl method
always returns one and only one executable path, even for an executable
with multiple links):
Fix DynamicLibraryTest.cpp on FreeBSD and NetBSD
Summary:
After rL301562, on FreeBSD the DynamicLibrary unittests fail, because
the test uses getMainExecutable("DynamicLibraryTests", Ptr), and since
the path does not contain any slashes, retrieving the main executable
will not work.
Reimplement getMainExecutable() for FreeBSD and NetBSD using sysctl(3),
which is more reliable than fiddling with relative or absolute paths.
Also add retrieval of the original argv[] from the GoogleTest framework,
to use as a fallback for other OSes.
Reid Kleckner [Wed, 17 May 2017 18:16:17 +0000 (18:16 +0000)]
Re-land r303274: "[CrashRecovery] Use SEH __try instead of VEH when available"
We have to check gCrashRecoveryEnabled before using __try.
In other words, SEH works too well and we ended up recovering from
crashes in implicit module builds that we weren't supposed to. Only
libclang is supposed to enable CrashRecoveryContext to allow implicit
module builds to crash.
Reid Kleckner [Wed, 17 May 2017 17:02:16 +0000 (17:02 +0000)]
[CrashRecovery] Use SEH __try instead of VEH when available
Summary:
It avoids problems when other libraries raise exceptions. In particular,
OutputDebugString raises an exception that the debugger is supposed to
catch and suppress. VEH kicks in first right now, and that is entirely
incorrect.
Unfortunately, GCC does not support SEH, so I've kept the old buggy VEH
codepath around. We could fix it with SetUnhandledExceptionFilter, but
that is not per-thread, so a well-behaved library shouldn't set it.
Zachary Turner [Wed, 17 May 2017 16:39:06 +0000 (16:39 +0000)]
[CodeView] Simplify the use of visiting type records & streams.
There is often a lot of boilerplate code required to visit a type
record or type stream. The #1 use case is that you have a sequence
of bytes that represent one or more records, and you want to
deserialize each one, switch on it, and call a callback with the
deserialized record that the user can examine. Currently this
requires at least 6 lines of code:
This is done by having the deserialization happen internally inside
of the visitTypeRecord function. Since this is occasionally not
desirable, the function provides a 3rd parameter that can be used
to change this behavior.
Hopefully this can significantly reduce the barrier to entry
to using the visitation infrastructure.
Zachary Turner [Wed, 17 May 2017 15:49:45 +0000 (15:49 +0000)]
[BitVector] Add find_[first,last]_[set,unset]_in.
A lot of code is duplicated between the first_last and the
next / prev methods. All of this code can be shared if they
are implemented in terms of find_first_in(Begin, End) etc,
in which case find_first = find_first_in(0, Size) and find_next
is find_first_in(Prev+1, Size), with similar reductions for
the other methods.
Sanjay Patel [Wed, 17 May 2017 14:21:19 +0000 (14:21 +0000)]
[InstCombine] add isCanonicalPredicate() helper function and use it; NFCI
There should be a slight efficiency improvement from handling icmp/fcmp with one matcher and reducing duplicated code.
The larger motivation is that there are questions about how predicate canonicalization is handled, and the refactoring
should make it easier if we want to change any of that behavior.
1. As noted in the code comment, we've chosen 3 of the 16 FCMP preds as not canonical. Why those 3? It goes back to
rL32751 from what I can tell, but I'm not sure if there's a justification for that rule.
2. We currently do not canonicalize integer select conditions. Should we use the same rule that applies to branches
for selects?
3. We currently do canonicalize some FP select conditions, and those rules would conflict with the rule shown here.
Should one or both be changed?
No-functional-change-intended, but adding tests anyway because there's no coverage for most of the predicates.
The variables MinGPR/MinG8R were not updated properly when resetting the
offsets, which in the included testcase lead to saving the CR register
in the same location as R30.
George Rimar [Wed, 17 May 2017 12:10:51 +0000 (12:10 +0000)]
[DWARF] - Cleanup relocations proccessing.
RelocAddrMap was a pair of <width, address>, where width is relocation size (4/8/x, x < 8),
and width field was never used in code.
Relocations proccessing loop had checks for width field. Does not look like DWARF parser
should do that. There is probably no much sense to validate relocations during proccessing
them in parser.
Patch removes relocation's width relative code from DWARFContext.
Diana Picus [Wed, 17 May 2017 08:57:28 +0000 (08:57 +0000)]
[GlobalISel][TableGen] Fix handling of default operands
When looping through a destination pattern's operands to decide how many
default operands we need to introduce, we used to count the "expanded"
number of operands. So if one default operand would be rendered as 2
values, we'd count it as 2 operands, when in fact it needs to count as
only 1 operand regardless of how many values it expands to.
This turns out to be a problem only in some very specific cases, e.g.
when we have one operand with multiple default values followed by more
operands with default values (see the new test). In such a situation
we'd stop looping before looking at all the operands, and then error out
assuming that we don't have enough default operands to make up the
shortfall.
At the moment this only affects ARM.
The patch removes the loop counting default operands entirely and
assumes that we'll have to introduce values for any default operand that
we find (i.e. we're assuming it cannot be given as a child at all). It
also extracts the code for adding renderers for default operands into a
helper method.
Summary:
Debug info sections, (or non-SHF_ALLOC sections in general) should be
linked as if their load address was zero to emulate the behavior of the
static linker.
This bug was discovered because it was breaking lldb expression evaluation on
linux.
Jonas Paulsson [Wed, 17 May 2017 07:36:03 +0000 (07:36 +0000)]
Make sure -optimize-regalloc=false is used correctly by user.
Don't allow -optimize-regalloc=false with -regalloc given for anything other
than 'fast'. The other register allocators depend on the supporting passes
added by addOptimizedRegAlloc().
Max Kazantsev [Wed, 17 May 2017 04:09:14 +0000 (04:09 +0000)]
[SCEV] Always sort AddRecExprs from different loops by dominance
Sorting of AddRecExprs by loop nesting does not make sense since we only invoke
the CompareSCEVComplexity for AddRecExprs that are used by one SCEV. This
guarantees that there is always a dominance relationship between them. This
patch removes the sorting by nesting which is a dead code in current usage of
this function.
Gor Nishanov [Wed, 17 May 2017 03:09:22 +0000 (03:09 +0000)]
[coroutines] Handle spills before catchswitch
If we need to spill the result of the PHI instruction, we insert the spill after
all of the PHIs and EHPads, however, in a catchswitch block there is no
room to insert the spill. Make room by splitting away catchswitch into a separate
block.
Zachary Turner [Tue, 16 May 2017 22:50:32 +0000 (22:50 +0000)]
[Support] Ignore OutputDebugString exceptions in our crash recovery.
Since we use AddVectoredExceptionHandler, we get notified of
every exception that gets raised by a program. Sometimes these
are not necessarily errors though, and this can be especially
true when linking against a library that we have no control
over, and may raise an exception internally which it intends
to catch.
In particular, the Windows API OutputDebugString does exactly
this. It raises an exception inside of a __try / __except,
giving the debugger a chance to handle the exception to print
the message to the debug console.
But this doesn't interoperate nicely with our vectored exception
handler, which just sees another exception and decides that we
need to terminate the program.
Add a special case for this so that we ignore ODS exceptions
and continue normally.
Note that a better fix is to simply not use vectored exception
handlers and use SEH instead, but given that MinGW doesn't support
SEH, this is the only solution for MinGW.
Adrian McCarthy [Tue, 16 May 2017 22:11:25 +0000 (22:11 +0000)]
Add test for FixedStreamArrayIterator::operator->
The operator-> implementation comes from iterator_facade_base, so it should
just work given that the iterator has a tested operator*. But r302257 showed
that required careful handling of for the const qualifier. This patch ensures
the fix in r302257 doesn't regress.
Sanjay Patel [Tue, 16 May 2017 21:51:04 +0000 (21:51 +0000)]
[InstSimplify] add folds for constant mask of value shifted by constant
We would eventually catch these via demanded bits and computing known bits in InstCombine,
but I think it's better to handle the simple cases as soon as possible as a matter of efficiency.
This fold allows further simplifications based on distributed ops transforms. eg:
%a = lshr i8 %x, 7
%b = or i8 %a, 2
%c = and i8 %b, 1
InstSimplify can directly fold this now:
%a = lshr i8 %x, 7
Easwaran Raman [Tue, 16 May 2017 21:18:09 +0000 (21:18 +0000)]
[Inliner] Do not mix callsite and callee hotness based updates.
Update threshold based on callee's hotness only when BFI is not available.
Otherwise use only callsite's hotness. This makes it easier to reason about
hotness related threshold updates.
The lowering pipeline is:
llvm.ppc.cfence in IR -> PPC::CFENCE8 in isel -> Actual instructions in
expandPostRAPseudo.
The reason why expandPostRAPseudo is chosen is because previous passes
are likely eliminating instructions like cmpw 3, 3 (early CSE) and bne-
7, .+4 (some branch pass(s)).
Easwaran Raman [Tue, 16 May 2017 20:14:39 +0000 (20:14 +0000)]
Add hasProfileSummary and has{Sample|Instrumentation}Profile methods
ProfileSummaryInfo already checks whether the module has sample profile
in determining profile counts. This will also be useful in inliner to
clean up threshold updates.
Nirav Dave [Tue, 16 May 2017 19:43:56 +0000 (19:43 +0000)]
Elide stores which are overwritten without being observed.
Summary:
In SelectionDAG, when a store is immediately chained to another store
to the same address, elide the first store as it has no observable
effects. This is causes small improvements dealing with intrinsics
lowered to stores.
Test notes:
* Many testcases overwrite store addresses multiple times and needed
minor changes, mainly making stores volatile to prevent the
optimization from optimizing the test away.
* Many X86 test cases optimized out instructions associated with
associated with va_start.
* Note that test_splat in CodeGen/AArch64/misched-stp.ll no longer has
dependencies to check and can probably be removed and potentially
replaced with another test.