Hal Finkel [Wed, 5 Aug 2015 07:45:28 +0000 (07:45 +0000)]
[MachineCombiner] Don't use the opcode-only form of computeInstrLatency
In r242277, I updated the MachineCombiner to work with itineraries, but I
missed a call that is scheduling-model-only (the opcode-only form of
computeInstrLatency). Using the form that takes an MI* allows this to work with
itineraries (and should be NFC for subtargets with scheduling models).
Davide Italiano [Wed, 5 Aug 2015 07:18:31 +0000 (07:18 +0000)]
[llvm-objdump] Call exit(1) on error, i.e. fail early.
Previously we kept going on partly corrupted input, which might result
in garbage being printed, or even worse, random crashes.
Rafael mentioned that this is the GNU behavior as well, but after some
discussion we both agreed it's probably better to emit a reasonable
error message and exit. As a side-effect of this commit, now we don't
rely on global state for error codes anymore. objdump was the last tool
in the toolchain which needed to be converted. Hopefully the old behavior
won't sneak into the tree again.
NAKAMURA Takumi [Wed, 5 Aug 2015 06:11:23 +0000 (06:11 +0000)]
unittests/ADT/ArrayRefTest.cpp: Suppress r243995 on g++-4.8 for now to unbreak bots.
For example of mingw-w64-g++-4.8.1,
llvm/unittests/ADT/ArrayRefTest.cpp: In member function 'virtual void {anonymous}::ArrayRefTest_AllocatorCopy_Test::TestBody()':
llvm/unittests/ADT/ArrayRefTest.cpp:56:40: internal compiler error: in count_type_elements, at expr.c:5523
} Array3Src[] = {{"hello"}, {"world"}};
^
Please submit a full bug report,
with preprocessed source if appropriate.
Temporarily revert r244012 while we see if it's really necessary.
Add a comment explaining the current theory as to why we'd need
the -lole32 on the link line.
Yaron Keren [Tue, 4 Aug 2015 15:57:04 +0000 (15:57 +0000)]
Avoid passing nullptr to std::equal.
As documented in the LLVM Coding Standards, indeed MSVC incorrectly asserts
on this in Debug mode. This happens when building clang with Visual C++ and
-triple i686-pc-windows-gnu on these clang regression tests:
Sanjay Patel [Tue, 4 Aug 2015 15:49:57 +0000 (15:49 +0000)]
wrap OptSize and MinSize attributes for easier and consistent access (NFCI)
Create wrapper methods in the Function class for the OptimizeForSize and MinSize
attributes. We want to hide the logic of "or'ing" them together when optimizing
just for size (-Os).
Currently, we are not consistent about this and rely on a front-end to always set
OptimizeForSize (-Os) if MinSize (-Oz) is on. Thus, there are 18 FIXME changes here
that should be added as follow-on patches with regression tests.
This patch is NFC-intended: it just replaces existing direct accesses of the attributes
by the equivalent wrapper call.
Sanjay Patel [Tue, 4 Aug 2015 15:21:56 +0000 (15:21 +0000)]
[x86] machine combiner reassociation: mark EFLAGS operand as 'dead'
In the commentary for D11660, I wasn't sure if it was alright to create new
integer machine instructions without also creating the implicit EFLAGS operand.
From what I can see, the implicit operand is always created by the MachineInstrBuilder
based on the instruction type, so we don't have to do that explicitly. However, in
reviewing the debug output, I noticed that the operand was not marked as 'dead'.
The machine combiner should do that to preserve future optimization opportunities
that may be checking for that dead EFLAGS operand themselves.
[mips][FastISel] Disable code generation for unsupported targets through FastISel.
Summary:
Previously, we would check whether the target is supported or not, only in
fastSelectInstruction(). This means that 64-bit targets could use FastISel too.
We fix this by checking every overridden method of the FastISel class and
by falling back to SelectionDAG if the target isn't supported. This change
should have been committed along with r243638, but somehow I missed it.
Revert r229675 - [mips] Avoid redundant sign extension of the result of binary bitwise instructions.
It introduced two regressions on 64-bit big-endian targets running under N32
(MultiSource/Benchmarks/tramp3d-v4/tramp3d-v4, and
MultiSource/Applications/kimwitu++/kc) The issue is that on 64-bit targets
comparisons such as BEQ compare the whole GPR64 but incorrectly tell the
instruction selector that they operate on GPR32's. This leads to the
elimination of i32->i64 extensions that are actually required by
comparisons to work correctly.
There's currently a patch under review that fixes this problem.
r243883 and r243961 made a use-after-free far more likely:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/6041/steps/check-llvm%20asan/logs/stdio
Unresolved nodes get inserted into the `Cycles` array. If they later
get resolved through RAUW, we need to update the reference. It's
interesting that this never hit before (maybe an asan-ified clang
bootstrap with `-flto -g` would have hit it, but I admit I haven't tried
anything quite that crazy).
Linker: Fix references to uniqued nodes after r243883
r243883 started moving 'distinct' nodes instead of duplicated them in
lib/Linker. This had the side-effect of sometimes not cloning uniqued
nodes that reference them. I missed a corner case:
!named = !{!0}
!0 = !{!1}
!1 = distinct !{!0}
!0 is the entry point for "remapping", and a temporary clone (say,
!0-temp) is created and mapped in case we need to model a uniquing
cycle.
Recursive descent into !1. !1 is distinct, so we leave it alone,
but update its operand to !0-temp.
Pop back out to !0. Its only operand, !1, hasn't changed, so we don't
need to use !0-temp. !0-temp goes out of scope, and we're finished
remapping, but we're left with:
Previously, if !0 and !0-temp ended up with identical operands, then
!0-temp couldn't have been referenced at all. Now that distinct nodes
don't get duplicated, that assumption is invalid. We need to
!0-temp->replaceAllUsesWith(!0) before freeing !0-temp.
I found this while running an internal `-flto -g` bootstrap. Strangely,
there was no case of this in the open source bootstrap I'd done before
commit...
Hal Finkel [Tue, 4 Aug 2015 06:29:12 +0000 (06:29 +0000)]
[SDAG] Fix a result chain in ExpandUnalignedLoad
On the code path in ExpandUnalignedLoad which expands an unaligned vector/fp
value in terms of a legal integer load of the same size, the ChainResult needs
to be the chain result of the integer load.
Chen Li [Tue, 4 Aug 2015 04:41:34 +0000 (04:41 +0000)]
Introduce enum value for previously defined metadata -- make.implicit
Summary: This patch adds enum value for an existing metadata type -- make.implicit. Using preassigned enum will be helpful to get compile time type checking and avoid string construction and comparison. The patch also changes uses of make.implicit from string metadata to enum metadata. There is no functionality change.
This adds the software division routines for the Windows RTABI. These are not
expected to be used often though as most modern Windows ARM capable targets
support hardware division. In the case that the target CPU doesnt support
hardware division, this will be the fallback.
[UB] Don't allocate space for contained types and then try to copy the
contained types into the space when we have no contained types. This
fixes the UB stemming from a call to memcpy with a null pointer. This
also reduces the calls to allocate because this actually happens in
a notable client - Clang.
Ahmed Bougacha [Tue, 4 Aug 2015 01:38:08 +0000 (01:38 +0000)]
[AArch64] Rename FP formats to be more consistent. NFC.
Some are named "FP", others "SD", others still "FP*SD".
Rename all this to just use "FP", which, except for conversions
(which don't use this format naming scheme), implies "SD" anyway.
[UB] Fix a nasty place where we would pass null pointers to memcpy.
This happens to work, but is not guaranteed to work. Indeed, most memcpy
interfaces in Linux-land annotate these arguments as nonnull, and GCC
and LLVM both can and do optimized based upon that. When they do so,
they might legitimately have miscompiled code calling this routine with
two valid iterators, 'nullptr' and 'nullptr'. There was even code doing
precisely this because StringRef().begin() and StringRef().end() both
produce null pointers.
Ahmed Bougacha [Tue, 4 Aug 2015 00:42:34 +0000 (00:42 +0000)]
[AArch64] Vector FCOPYSIGN supports Custom-lowering: mark it as such.
There's a bunch of code in LowerFCOPYSIGN that does smart lowering, and
is actually already vector-aware; let's use it instead of scalarizing!
The only interesting change is that for v2f32, we previously always used
use v4i32 as the integer vector type.
Use v2i32 instead, and mark FCOPYSIGN as Custom.
Ahmed Bougacha [Tue, 4 Aug 2015 00:32:55 +0000 (00:32 +0000)]
[CodeGen] Fix FCOPYSIGN legalization to account for mismatched types.
We used to legalize it like it's any other binary operations. It's not,
because it accepts mismatched operand types. Because of that, we used
to hit various asserts and miscompiles.
Specialize vector legalizations to, in the worst case, unroll, or, when
possible, to just legalize the operand that needs legalization.
Scalarization isn't covered, because I can't think of a target where
some but not all of the 1-element vector types are to be scalarized.
David Blaikie [Mon, 3 Aug 2015 22:30:24 +0000 (22:30 +0000)]
-Wdeprecated-clean: Fix cases of violating the rule of 5 in ways that are deprecated in C++11
Various value handles needed to be copy constructible and copy
assignable (mostly for their use in DenseMap). But to avoid an API that
might allow accidental slicing, make these members protected in the base
class and make derived classes final (the special members become
implicitly public there - but disallowing further derived classes that
might be sliced to the intermediate type).
Might be worth having a warning a bit like -Wnon-virtual-dtor that
catches public move/copy assign/ctors in classes with virtual functions.
(suppressable in the same way - by making them protected in the base,
and making the derived classes final) Could be fancier and only diagnose
them when they're actually called, potentially.
Also allow a few default implementations where custom implementations
(especially with non-standard return types) were implemented.
[Unroll] Improve the brute force loop unroll estimate by propagating
through PHI nodes across iterations.
This patch teaches the new advanced loop unrolling heuristics to propagate
constants into the loop from the preheader and around the backedge after
simulating each iteration. This lets us brute force solve simple recurrances
that aren't modeled effectively by SCEV. It also makes it more clear why we
need to process the loop in-order rather than bottom-up which might otherwise
make much more sense (for example, for DCE).
This came out of an attempt I'm making to develop a principled way to account
for dead code in the unroll estimation. When I implemented
a forward-propagating version of that it produced incorrect results due to
failing to propagate *cost* between loop iterations through the PHI nodes, and
it occured to me we really should at least propagate simplifications across
those edges, and it is quite easy thanks to the loop being in canonical and
LCSSA form.
Renato Golin [Mon, 3 Aug 2015 20:19:35 +0000 (20:19 +0000)]
[Release Script] Check for correct symlink name
While checking for the existence of the clang-tools-extra directory,
the script was not checking for its destination name, "extra", and
the script was failing when re-running without checking out new
sources.
David Blaikie [Mon, 3 Aug 2015 20:12:58 +0000 (20:12 +0000)]
-Wdeprecated-clean: Fix cases of violating the rule of 5 in ways that are deprecated in C++11
Some functions return concrete ByteStreamers by value - explicitly
support that in the base class. (dtor can be virtual, no one seems to be
polymorphically owning/destroying them)
David Blaikie [Mon, 3 Aug 2015 20:08:41 +0000 (20:08 +0000)]
Recommit r243824: -Wdeprecated-clean: Fix cases of violating the rule of 5 in ways that are deprecated in C++11
This reverts commit r243888, recommitting r243824.
This broke the Windows build due to a difference in the C++ standard
library implementation. Using emplace/forward_as_tuple should ensure
there's no need to copy ValIDs.
Derek Schuff [Mon, 3 Aug 2015 18:01:50 +0000 (18:01 +0000)]
Fix testing for end of stream in bitstream reader.
This fixes a bug found while working on the bitcode reader. In
particular, the method BitstreamReader::AtEndOfStream doesn't always
behave correctly when processing a data streamer. The method
fillCurWord doesn't properly set CurWord/BitsInCurWord if the data
streamer was already at eof, but GetBytes had not yet set the
ObjectSize field of the streaming memory object.
This patch fixes this problem, and provides a test to show that
this problem has been fixed.
Since r241097, `DIBuilder` has only created distinct `DICompileUnit`s.
The backend is liable to start relying on that (if it hasn't already),
so make uniquable `DICompileUnit`s illegal and automatically upgrade old
bitcode. This is a nice cleanup, since we can remove an unnecessary
`DenseSet` (and the associated uniquing info) from `LLVMContextImpl`.
Almost all the testcases were updated with this script:
Tim Northover [Mon, 3 Aug 2015 17:20:10 +0000 (17:20 +0000)]
ARM: prefer allocating VFP regs at stride 4 on Darwin.
This is necessary for WatchOS support, where the compact unwind format assumes
this kind of layout. For now we only want this on Swift-like CPUs though, where
it's been the Xcode behaviour for ages. Also, since it can expand the prologue
we don't want it at -Oz.
Instead of cloning distinct `MDNode`s when linking in a module, just
move them over. The module linker destroys the source module, so the
old node would otherwise just be leaked on the context. Create the new
node in place. This also reduces the number of cloned uniqued nodes
(since it's less likely their operands have changed).
This mapping strategy is only correct when we're discarding the source,
so the linker turns it on via a ValueMapper flag, `RF_MoveDistinctMDs`.
There's nothing observable in terms of `llvm-link` output here: the
linked module should be semantically identical.
I'll be adding more 'distinct' nodes to the debug info metadata graph in
order to break uniquing cycles, so the benefits of this will partly come
in future commits. However, we should get some gains immediately, since
we have a fair number of 'distinct' `DILocation`s being linked in.
JF Bastien [Mon, 3 Aug 2015 15:29:47 +0000 (15:29 +0000)]
Refactor AtomicExpand::expandAtomicRMWToCmpXchg into a standalone function.
Summary:
This is useful for PNaCl's `RewriteAtomics` pass. NaCl intrinsics don't exist for some of the more exotic RMW instructions, so by refactoring this function into its own, `RewriteAtomics` can share code rewriting those atomics with `AtomicExpand` while additionally saving a few cycles by generating the `cmpxchg` NaCl-specific intrinsic with the callback. Without this patch, `RewriteAtomics` would require two extra passes over functions, by first requiring use of the full `AtomicExpand` pass to just expand the leftover exotic RMWs and then running itself again to expand resulting `cmpxchg`s.
Artur Pilipenko [Mon, 3 Aug 2015 14:31:49 +0000 (14:31 +0000)]
Currently string attributes on function arguments/return values can be generated using LLVM API. However they are not supported in parser. So, the following scenario will fail:
* generate function with string attribute using API,
* dump it in LL format,
* try to parse.
Add parser support for string attributes to fix the issue.
Silviu Baranga [Mon, 3 Aug 2015 14:00:58 +0000 (14:00 +0000)]
[TTI] Fix default costs for interleaved accesses
Summary:
Modify the cost calculation function for interleaved accesses
to use the target-specific costs for insert/extract element and
memory operations.
This better models the case where the backend can't match
the interleaved group, and we are forced to use a wide load
and shuffle vectors.
Interleaved accesses are not enabled by default, so this shouldn't
cause a performance change.
John Brawn [Mon, 3 Aug 2015 12:13:33 +0000 (12:13 +0000)]
[ARM] Make GlobalMerge merge extern globals by default
Enabling merging of extern globals appears to be generally either beneficial or
harmless. On some benchmarks suites (on Cortex-M4F, Cortex-A9, and Cortex-A57)
it gives improvements in the 1-5% range, but in the rest the overall effect is
zero.
John Brawn [Mon, 3 Aug 2015 12:08:41 +0000 (12:08 +0000)]
[GlobalMerge] Allow targets to enable merging of extern variables, NFC.
Adjust the GlobalMergeOnExternal option so that the default behaviour is to
do whatever the Target thinks is best. Explicitly enabled or disabling the
option will override this default.
The test/DebugInfo/dwarfdump-macho-universal.test test added in r243862 uses
an input from another test's directory (test/tools/dsymutil/Inputs/fat-test.o)
which breaks our test setup.
Copying the required test input to the test's Input directory to fix the issue.
James Molloy [Mon, 3 Aug 2015 09:24:48 +0000 (09:24 +0000)]
Be less conservative about forming IT blocks.
In http://reviews.llvm.org/rL215382, IT forming was made more conservative under
the belief that a flag-setting instruction was unpredictable inside an IT block on ARMv6M.
But actually, ARMv6M doesn't even support IT blocks so that's impossible. In the ARMARM for
v7M, v7AR and v8AR it states that the semantics of such an instruction changes inside an
IT block - it doesn't set the flags. So actually it is fine to use one inside an IT block
as long as the flags register is dead afterwards.
This gives significant performance improvements in a variety of MPEG based workloads.
ValueMapper: Only check for cycles if operands change
This is a minor optimization to only check for unresolved operands
inside `mapDistinctNode()` if the operands have actually changed. This
shouldn't really cause any change in behaviour. I didn't actually see a
slowdown in a profile, I was just poking around nearby and saw the
opportunity.
JF Bastien [Mon, 3 Aug 2015 00:00:11 +0000 (00:00 +0000)]
WebAssembly: implement getScalarShiftAmountTy so we can shift by amount, with type
Summary: This currently sets the shift amount RHS to the same type as the LHS, and assumes that the LHS is a simple type. This isn't currently the case e.g. with weird integers sizes, but will eventually be true and will assert if not. That's what you get for having an experimental backend: break it and you get to keep both pieces. Most backends either set the RHS to MVT::i32 or MVT::i64, but WebAssembly is a virtual ISA and tries to have regular-looking binary operations where both operands are the same type (even if a 64-bit RHS shifter is slightly silly, hey it's free!).
Change `DIELoc` and `DIEBlock` to stop inheriting from `DIE`, instead
inheriting from `DIEValueList` to share the value storage API. This
awkward bit of code-sharing was also fairly confusing: neither `DIELoc`
nor `DIEBlock` represents a `DIE`, so why would they inherit from it?
Aside from the API cleanup, this should improve debug info memory usage
in the backend, since it shaves five pointers off of every `DIELoc` and
`DIEBlock`. I haven't bothered to measure the savings, though.
AsmPrinter: Split out non-DIE printing from DIE::print(), NFC
Split out a helper `printValues()` for printing `DIEBlock` and `DIELoc`,
instead of relying on `DIE::print()`. The shared code was actually
fairly small there. No functionality change intended.
AsmPrinter: Change DIEValueList to a subclass of DIE, NFC
Rewrite `DIEValueList` as a subclass of `DIE`, renaming its API to match
`DIE`'s. This is preparation for changing `DIEBlock` and `DIELoc` to
stop inheriting from `DIE` and inherit directly from `DIEValueList`.
I thought about leaving this as a has-a relationship (and changing
`DIELoc` and `DIEBlock` to also have-a `DIEValueList`), but that seemed
to require a fair bit more boilerplate and I think it needed more
changes to the `DwarfUnit` API than this will.
projects / llvm / log