Zachary Turner [Thu, 4 May 2017 23:53:54 +0000 (23:53 +0000)]
[pdb] Don't verify TPI hash values up front.
Verifying the hash values as we are currently doing
results in iterating every type record before the user
even tries to access the first one, and the API user
has no control over, or ability to hook into this
process.
As a result, when the user wants to iterate over types
to print them or index them, this results in a second
iteration over the same list of types. When there's
upwards of 1,000,000 type records, this is obviously
quite undesirable.
This patch raises the verification outside of TpiStream
, and llvm-pdbdump hooks a hash verification visitor
into the normal dumping process. So we still verify
the hash records, but we can do it while not requiring
a second iteration over the type stream.
Zachary Turner [Thu, 4 May 2017 23:53:29 +0000 (23:53 +0000)]
[PDB] Don't build the entire source file list up front.
I tried to run llvm-pdbdump on a very large (~1.5GB) PDB to
try and identify show-stopping performance problems. This
patch addresses the first such problem.
When loading the DBI stream, before anyone has even tried to
access a single record, we build an in memory map of every
source file for every module. In the particular PDB I was
using, this was over 85 million files. Specifically, the
complexity is O(m*n) where m is the number of modules and
n is the average number of source files (including headers)
per module.
The whole reason for doing this was so that we could have
constant time access to any module and any of its source
file lists. However, we can still get O(1) access to the
source file list for a given module with a simple O(m)
precomputation, and access to the list of modules is
already O(1) anyway.
So this patches reduces the O(m*n) up-front precomputation
to an O(m) one, where n is ~6,500 and n*m is about 85 million
in my pathological test case.
Zachary Turner [Thu, 4 May 2017 23:53:01 +0000 (23:53 +0000)]
[llvm-pdbdump] Only build the TypeDatabase if necessary.
Building the type database is expensive, and can take multiple
minutes for large PDBs. But we only need it in certain cases
depending on what command line options are specified. So only
build it when we know we're about to need it.
Craig Topper [Thu, 4 May 2017 21:45:49 +0000 (21:45 +0000)]
[JumpThreading] When processing compares, explicitly check that the result type is not a vector rather than check for it being an integer.
Compares always return a scalar integer or vector of integers. isIntegerTy returns false for vectors, but that's not completely obvious. So using isVectorTy is less confusing.
Sanjay Patel [Thu, 4 May 2017 19:51:34 +0000 (19:51 +0000)]
[InstSimplify] add folds for or-of-casted-icmps
The sibling folds for 'and' with casts were added with https://reviews.llvm.org/rL273200.
This is a preliminary step for adding the 'or' variants for the folds added with https://reviews.llvm.org/rL301260.
The reason for the strange form with constant LHS in the 1st test is because there's another missing fold in that
case for the inverted predicate. That should be fixed when we add the ConstantRange functionality for 'or-of-icmps'
that already exists for 'and-of-icmps'.
I'm hoping to share more code for the and/or cases, so we won't have these differences. This will allow us to remove
code from InstCombine. It's also possible that we can remove some code here in InstSimplify. I think we have some
duplicated folds because patterns are not matched in a general way.
Reid Kleckner [Thu, 4 May 2017 18:19:52 +0000 (18:19 +0000)]
[ms-inline-asm] Use the frontend size only for ambiguous instructions
This avoids problems on code like this:
char buf[16];
__asm {
movups xmm0, [buf]
mov [buf], eax
}
The frontend size in this case (1) is wrong, and the register makes the
instruction matching unambiguous. There are also enough bytes available
that we shouldn't complain to the user that they are potentially using
an incorrectly sized instruction to access the variable.
Craig Topper [Thu, 4 May 2017 17:00:41 +0000 (17:00 +0000)]
[APInt] Reduce number of allocations involved in multiplying. Reduce worst case multiply size
Currently multiply is implemented in operator*=. Operator* makes a copy and uses operator*= to modify the copy.
Operator*= itself allocates a temporary buffer to hold the multiply result as it computes it. Then copies it to the buffer in *this.
Operator*= attempts to bound the size of the result based on the number of active bits in its inputs. It also has a couple special cases to handle 0 inputs without any memory allocations or multiply operations. The best case is that it calculates a single word regardless of input bit width. The worst case is that it calculates the a 2x input width result and drop the upper bits.
Since operator* uses operator*= it incurs two allocations, one for a copy of *this and one for the temporary allocation. Neither of these allocations are kept after the method operation is done.
The main usage in the backend appears to be ConstantRange::multiply which uses operator* rather than operator*=.
This patch moves the multiply operation to operator* and implements operator*= using it. This avoids the copy in operator*. operator* now allocates a result buffer sized the same width as its inputs no matter what. This buffer will be used as the buffer for the returned APInt. Finally, we reuse tcMultiply to implement the multiply operation. This function is capable of not calculating additional upper words that will be discarded.
This change does lose the special optimizations for the inputs using less words than their size implies. But it also removed the getActiveBits calls from all multiplies. If we think those optimizations are important we could look at providing additional bounds to tcMultiply to limit the computations.
Jonas Paulsson [Thu, 4 May 2017 13:33:30 +0000 (13:33 +0000)]
[SystemZ] Make copyPhysReg() add impl-use operands of super reg.
When a 128 bit COPY is lowered into two instructions, an impl-use operand of
the super-reg should be added to each new instruction in case one of the
sub-regs is undefined.
In this patch, I introduce a new altmacro string delimiter.
This review is the second review in a series of four reviews.
(one for each altmacro feature: LOCAL, string delimiter, string '!' escape sign and absolute expression as a string '%' ).
In the alternate macro mode, you can delimit strings with matching angle brackets <..>
when using it as a part of calling macro arguments.
As described in the https://sourceware.org/binutils/docs-2.27/as/Altmacro.html
"<string>
You can delimit strings with matching angle brackets."
assumptions:
1. If an argument begins with '<' and ends with '>'. The argument is considered as a string.
2. Except adding new string mark '<..>', a regular macro behavior is expected.
3. The altmacro cannot affect the regular less/greater behavior.
4. If a comma is present inside an angle brackets it considered as a character and not as a separator.
Sam Parker [Thu, 4 May 2017 07:31:28 +0000 (07:31 +0000)]
[ARM] ACLE Chapter 9 intrinsics
Added the integer data processing intrinsics from ACLE v2.1 Chapter 9
but I have missed out the saturation_occurred intrinsics for now. For
the instructions that read and write the GE bits, a chain is included
and the only instruction that reads these flags (sel) is only
selectable via the implemented intrinsic.
Oren Ben Simhon [Thu, 4 May 2017 07:22:49 +0000 (07:22 +0000)]
[X86] Disabling PLT in Regcall CC Functions
According to psABI, PLT stub clobbers XMM8-XMM15.
In Regcall calling convention those registers are used for passing parameters.
Thus we need to prevent lazy binding in Regcall.
This makes it simpler for the runtime to consistently handle the entries
in the function sled index in both 32 and 64 bit platforms where the
XRay runtime works.
Summary:
This change adds a new section to the xray-instrumented binary that
stores an index into ranges of the instrumentation map, where sleds
associated with the same function can be accessed as an array. At
runtime, we can get access to this index by function ID offset allowing
for selective patching and unpatching by function ID.
Each entry in this new section (xray_fn_idx) will include two pointers
indicating the start and one past the end of the sleds associated with
the same function. These entries will be 16 bytes long on x86 and
aarch64. On arm, we align to 16 bytes anyway so the runtime has to take
that into consideration.
__{start,stop}_xray_fn_idx will be the symbols that the runtime will
look for when we implement the selective patching/unpatching by function
id APIs. Because XRay synthesizes the function id's in a monotonically
increasing manner at runtime now, implementations (and users) can use
this table to look up the sleds associated with a specific function.
This is useful in implementations that want to do things like:
- Implement coverage mode for functions by patching everything
pre-main, then as functions are encountered, the installed handler
can unpatch the function that's been encountered after recording
that it's been called.
- Do "learning mode", so that the implementation can figure out some
statistical information about function calls by function id for a
time being, and then determine which functions are worth
uninstrumenting at runtime.
- Do "selective instrumentation" where an implementation can
specifically instrument only certain function id's at runtime
(either based on some external data, or through some other
heuristics) instead of patching all the instrumented functions at
runtime.
IR: Use pointers instead of GUIDs to represent edges in the module summary. NFCI.
When profiling a no-op incremental link of Chromium I found that the functions
computeImportForFunction and computeDeadSymbols were consuming roughly 10% of
the profile. The goal of this change is to improve the performance of those
functions by changing the map lookups that they were previously doing into
pointer dereferences.
This is achieved by changing the ValueInfo data structure to be a pointer to
an element of the global value map owned by ModuleSummaryIndex, and changing
reference lists in the GlobalValueSummary to hold ValueInfos instead of GUIDs.
This means that a ValueInfo will take a client directly to the summary list
for a given GUID.
Summary:
This is an implementation of the loop detection logic that XRay needs to
determine whether a function might take time at runtime. Without this
heuristic, XRay will tend to not instrument short functions that have
loops that might have runtime dependent on inputs or external values.
While this implementation doesn't do any further analysis than just
figuring out whether there is a loop in the MachineFunction being
code-gen'ed, we're paving the way for being able to perform more
sophisticated analysis of the function in the future (for example to
determine whether the trip count for the loop might be constant, and
make a decision on that instead). This enables us to cover more
functions with the default heuristics, and potentially identify ones
that have variable runtime latency just by looking for the presence of
loops.
[SCEV] createAddRecFromPHI: Optimize for the most common case.
Summary:
The existing implementation creates a symbolic SCEV expression every
time we analyze a phi node and then has to remove it, when the analysis
is finished. This is very expensive, and in most of the cases it's also
unnecessary. According to the data I collected, ~60-70% of analyzed phi
nodes (measured on SPEC) have the following form:
PN = phi(Start, OP(Self, Constant))
Handling such cases separately significantly speeds this up.
Craig Topper [Wed, 3 May 2017 23:12:29 +0000 (23:12 +0000)]
[KnownBits] Add methods for determining if KnownBits is a constant value
This patch adds isConstant and getConstant for determining if KnownBits represents a constant value and to retrieve the value. Use them to simplify code.
Craig Topper [Wed, 3 May 2017 22:25:19 +0000 (22:25 +0000)]
[ValueTracking] Remove handling for BitWidth being 0 in ComputeSignBit and isKnownNonZero.
I don't believe its possible to have non-zero values here since DataLayout became required. The APInt constructor inside of the KnownBits object will assert if this ever happens.
Sanjay Patel [Wed, 3 May 2017 21:55:34 +0000 (21:55 +0000)]
[TargetLowering] use isSubsetOf in SimplifyDemandedBits; NFCI
This is the DAG equivalent of https://reviews.llvm.org/D32255 ,
which will hopefully be committed again. The functionality
(preferring a 'not' op) is already here in the DAG, so this is
just intended to be a clean-up and performance improvement.
DebugInfo: elide type index entries for synthetic types
Compiler emitted synthetic types may not have an associated DIFile
(translation unit). In such a case, when generating CodeView debug type
information, we would attempt to compute an absolute filepath which
would result in a segfault due to a NULL DIFile*. If there is no source
file associated with the type, elide the type index entry for the type
and record the type information. This actually results in higher
fidelity debug information than clang/C2 as of this writing.
Ahmed Bougacha [Wed, 3 May 2017 20:33:58 +0000 (20:33 +0000)]
[AArch64] Make the TargetParser add CPU exts provided by the arch.
Otherwise, each CPU has to manually specify the extensions it supports,
even though they have to be a superset of the base arch extensions.
And when there's redundant data there's stale data, so most of the CPUs
lie about the features they support (almost none lists AEK_FP).
Instead, do the saner thing: add the optional extensions on top of the
base extensions provided by the architecture.
Greg Clayton [Wed, 3 May 2017 18:25:46 +0000 (18:25 +0000)]
Break verification down into smaller functions to keep code clean.
Adrian requested that we break things down to make things clean in the DWARFVerifier. This patch breaks everything down into nice individual functions and cleans up the code quite a bit and prepares us for the next round of verifiers.
Reid Kleckner [Wed, 3 May 2017 18:17:31 +0000 (18:17 +0000)]
[IR] Abstract away ArgNo+1 attribute indexing as much as possible
Summary:
Do three things to help with that:
- Add AttributeList::FirstArgIndex, which is an enumerator currently set
to 1. It allows us to change the indexing scheme with fewer changes.
- Add addParamAttr/removeParamAttr. This just shortens addAttribute call
sites that would otherwise need to spell out FirstArgIndex.
- Remove some attribute-specific getters and setters from Function that
take attribute list indices. Most of these were only used from
BuildLibCalls, and doesNotAlias was only used to test or set if the
return value is malloc-like.
I'm happy to split the patch, but I think they are probably easier to
review when taken together.
This patch should be NFC, but it sets the stage to change the indexing
scheme to this, which is more convenient when indexing into an array:
0: func attrs
1: retattrs
2...: arg attrs
Anna Thomas [Wed, 3 May 2017 17:43:59 +0000 (17:43 +0000)]
[RuntimeLoopUnroller] Add assert that we dont unroll non-rotated loops
Summary:
Cloning basic blocks in the loop for runtime loop unroller depends on loop being
in rotated form (i.e. loop latch target is the exit block).
Assert that this is true, so that callers of runtime loop unroller pass in
canonical loops.
The single caller of this function has that check recently added:
https://reviews.llvm.org/rL301239
Zachary Turner [Wed, 3 May 2017 17:11:40 +0000 (17:11 +0000)]
[CodeView] Use actual strings for dealing with checksums and lines.
The raw CodeView format references strings by "offsets", but it's
confusing what table the offset refers to. In the case of line
number information, it's an offset into a buffer of records,
and an indirection is required to get another offset into a
different table to find the final string. And in the case of
checksum information, there is no indirection, and the offset
refers directly to the location of the string in another buffer.
This would be less confusing if we always just referred to the
strings by their value, and have the library be smart enough
to correctly resolve the offsets on its own from the right
location.
This patch makes that possible. When either reading or writing,
all the user deals with are strings, and the library does the
appropriate translations behind the scenes.
Zachary Turner [Wed, 3 May 2017 17:11:11 +0000 (17:11 +0000)]
[llvm-readobj] Update readobj to re-use parsing code.
llvm-readobj hand rolls some CodeView parsing code for string
tables, so this patch updates it to re-use some of the newly
introduced parsing code in LLVMDebugInfoCodeView.
Tim Northover [Wed, 3 May 2017 16:54:30 +0000 (16:54 +0000)]
ARM: add extra test for addrmode folding.
I was worried we might replace a mul with a mul+shift even if there were later
uses. Turns out to be unfounded but I'd just as well add an actual test for it.
Greg Clayton [Wed, 3 May 2017 16:02:29 +0000 (16:02 +0000)]
Create DWARFVerifier.cpp and .h and move all DWARF verification code over into it.
Adrian requested we create a DWARFVerifier.cpp file to contain all of the DWARF verification stuff. This change simply moves the functionality over into DWARFVerifier.h and DWARFVerifier.cpp, renames the DWARFVerifier methods to start with lower case, and switches DWARFContext.cpp over to using the new functionality.
Zachary Turner [Wed, 3 May 2017 15:58:37 +0000 (15:58 +0000)]
Resubmit r301986 and r301987 "Add codeview::StringTable"
This was reverted due to a "missing" file, but in reality
what happened was that I renamed a file, and then due to
a merge conflict both the old file and the new file got
added to the repository. This led to an unused cpp file
being in the repo and not referenced by any CMakeLists.txt
but #including a .h file that wasn't in the repo. In an
even more unfortunate coincidence, CMake didn't report the
unused cpp file because it was in a subdirectory of the
folder with the CMakeLists.txt, and not in the same directory
as any CMakeLists.txt.
The presence of the unused file was then breaking certain
tools that determine file lists by globbing rather than
by what's specified in CMakeLists.txt
In any case, the fix is to just remove the unused file from
the patch set.
Craig Topper [Wed, 3 May 2017 15:46:24 +0000 (15:46 +0000)]
[APInt] Give the value union a name so we can remove assumptions on VAL being the larger member
Currently several places assume the VAL member is always at least the same size as pVal. In particular for a memcpy in the move assignment operator. While this is a true assumption, it isn't good practice to assume this.
This patch gives the union a name so we can write the memcpy in terms of the union itself. This also adds a similar memcpy to the move constructor where we previously just copied using VAL directly.
This patch is mostly just a mechanical addition of the U in front of VAL and pVAL everywhere. But several constructors had to be modified since we can't directly initializer a field of named union from the initializer list.
[Hexagon] Adjust latency between allocframe and the first store on stack
Allocframe and the following stores on the stack have a latency of 2 cycles
when not in the same packet. This happens because R29 is needed early by the
store instruction. Since one of such stores can be packetized along with
allocframe and use old value of R29, we can assign it 0 cycle latency
while leaving latency of other stores to the default value of 2 cycles.