Commit Graph

1387 Commits

Author SHA1 Message Date
Benjamin Kramer efcf06f5f2 Move symbols from the global namespace into (anonymous) namespaces. NFC.
llvm-svn: 294837
2017-02-11 11:06:55 +00:00
Peter Collingbourne fa3175f2f6 Address Mehdi's post-commit review comments on r294795.
llvm-svn: 294822
2017-02-11 03:19:22 +00:00
Peter Collingbourne be9ffaacfa IR: Function summary extensions for whole-program devirtualization pass.
The summary information includes all uses of llvm.type.test and
llvm.type.checked.load intrinsics that can be used to devirtualize calls,
including any constant arguments for virtual constant propagation.

Differential Revision: https://reviews.llvm.org/D29734

llvm-svn: 294795
2017-02-10 22:29:38 +00:00
Adrian Prantl a5bf2d7003 Fix bitcode upgrade for DIGlobalVariables with a var: field.
This is a follow-up to https://reviews.llvm.org/D29349.  It turns out
that NeedUpgradeToDIGlobalVariableExpression is always necessary when
we encountered a version==0 record because it may always be referenced
via a list of globals in a DICompileUnit. My tests weren't good enough
to catch this though. To trigger this case, we need much older bitcode
produced by LLVM around version 3.7.

<rdar://problem/30404262>

Differential Revision: https://reviews.llvm.org/D29693

llvm-svn: 294488
2017-02-08 17:44:43 +00:00
Adrian Prantl e37d314464 Fix the bitcode upgrade for DIGlobalVariable in a DIImportedEntity context.
The bitcode upgrade for DIGlobalVariable unconditionally wrapped
DIGlobalVariables in a DIGlobalVariableExpression. When a
DIGlobalVariable is referenced by a DIImportedEntity, however, this is
wrong. This patch fixes the bitcode upgrade by deferring the creation
of DIGlobalVariableExpressions until we know the context of the
DIGlobalVariable.

<rdar://problem/30134279>

Differential Revision: https://reviews.llvm.org/D29349

llvm-svn: 294318
2017-02-07 17:35:41 +00:00
Mehdi Amini 1380edf4ef Revert "[ThinLTO] Add an auto-hide feature"
This reverts commit r293970.

After more discussion, this belongs to the linker side and
there is no added value to do it at this level.

llvm-svn: 293993
2017-02-03 07:41:43 +00:00
Mehdi Amini b0a8ff71e5 [ThinLTO] Add an auto-hide feature
When a symbol is not exported outside of the
DSO, it is can be hidden. Usually we try to internalize
as much as possible, but it is not always possible, for
instance a symbol can be referenced outside of the LTO
unit, or there can be cross-module reference in ThinLTO.

This is a recommit of r293912 after fixing build failures,
and a recommit of r293918 after fixing LLD tests.

Differential Revision: https://reviews.llvm.org/D28978

llvm-svn: 293970
2017-02-03 00:32:38 +00:00
Mehdi Amini 21c89dc920 Revert "[ThinLTO] Add an auto-hide feature"
This reverts commit r293918, one lld test does not pass.

llvm-svn: 293961
2017-02-02 23:20:36 +00:00
Mehdi Amini 97624fb1ec [ThinLTO] Add an auto-hide feature
When a symbol is not exported outside of the
DSO, it is can be hidden. Usually we try to internalize
as much as possible, but it is not always possible, for
instance a symbol can be referenced outside of the LTO
unit, or there can be cross-module reference in ThinLTO.

This is a recommit of r293912 after fixing build failures.

Differential Revision: https://reviews.llvm.org/D28978

llvm-svn: 293918
2017-02-02 18:31:35 +00:00
Mehdi Amini 827600deaf Revert "[ThinLTO] Add an auto-hide feature"
This reverts r293912, bots are broken.

llvm-svn: 293914
2017-02-02 18:24:37 +00:00
Mehdi Amini dc5a7444f0 [ThinLTO] Add an auto-hide feature
When a symbol is not exported outside of the
DSO, it is can be hidden. Usually we try to internalize
as much as possible, but it is not always possible, for
instance a symbol can be referenced outside of the LTO
unit, or there can be cross-module reference in ThinLTO.

Differential Revision: https://reviews.llvm.org/D28978

llvm-svn: 293912
2017-02-02 18:13:46 +00:00
Dehao Chen 0944a8c2ec Change debug-info-for-profiling from a TargetOption to a function attribute.
Summary: LTO requires the debug-info-for-profiling to be a function attribute.

Reviewers: echristo, mehdi_amini, dblaikie, probinson, aprantl

Reviewed By: mehdi_amini, dblaikie, aprantl

Subscribers: aprantl, probinson, ahatanak, llvm-commits, mehdi_amini

Differential Revision: https://reviews.llvm.org/D29203

llvm-svn: 293833
2017-02-01 22:45:09 +00:00
Matthias Braun 8c209aa877 Cleanup dump() functions.
We had various variants of defining dump() functions in LLVM. Normalize
them (this should just consistently implement the things discussed in
http://lists.llvm.org/pipermail/cfe-dev/2014-January/034323.html

For reference:
- Public headers should just declare the dump() method but not use
  LLVM_DUMP_METHOD or #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
- The definition of a dump method should look like this:
  #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
  LLVM_DUMP_METHOD void MyClass::dump() {
    // print stuff to dbgs()...
  }
  #endif

llvm-svn: 293359
2017-01-28 02:02:38 +00:00
Ivan Krasin c05c9db364 Avoid using unspecified ordering in MetadataLoader::MetadataLoaderImpl::parseOneMetadata.
Summary:
MetadataLoader::MetadataLoaderImpl::parseOneMetadata uses
the following construct in a number of places:

```
MetadataList.assignValue(<...>, NextMetadataNo++);
```

There, NextMetadataNo gets incremented, and since the order
of arguments evaluation is not specified, that can happen
before or after other arguments are evaluated.

In a few cases the other arguments indirectly use NextMetadataNo.
For instance, it's

```
MetadataList.assignValue(
    GET_OR_DISTINCT(DIModule,
                    (Context, getMDOrNull(Record[1]),
                     getMDString(Record[2]), getMDString(Record[3]),
                     getMDString(Record[4]), getMDString(Record[5]))),
    NextMetadataNo++);
```

getMDOrNull calls getMD that uses NextMetadataNo:

```
MetadataList.getMetadataFwdRef(NextMetadataNo);
```

Therefore, the order of evaluation becomes important. That caused
a very subtle LLD crash that only happens if compiled with GCC or
if LLD is built with LTO. In the case if LLD is compiled with Clang
and regular linking mode, everything worked as intended.

This change extracts incrementing of NextMetadataNo outside of
the arguments list to guarantee the correct order of evaluation.

For the record, this has taken 3 days to track to the origin. It all
started with a ThinLTO bot in Chrome not being able to link a target
if debug info is enabled.

Reviewers: pcc, mehdi_amini

Reviewed By: mehdi_amini

Subscribers: aprantl, llvm-commits

Differential Revision: https://reviews.llvm.org/D29204

llvm-svn: 293291
2017-01-27 15:54:49 +00:00
Mehdi Amini 3bb4d01db9 [ThinLTO] Fix lazy-loading of MDString instruction attachments
CFI is using intrinsics that takes MDString as arguments, and this
was broken during lazy-loading of metadata.

Differential Revision: https://reviews.llvm.org/D28916

llvm-svn: 292641
2017-01-20 20:29:16 +00:00
Mehdi Amini 2737989245 Add an assertion to PlaceholderQueue destructor, ensuring it has been flushed
llvm-svn: 292597
2017-01-20 10:18:32 +00:00
Mehdi Amini 67d2cc1fad [ThinLTO] Add a recursive step in Metadata lazy-loading
Summary:
Without this, we're stressing the RAUW of unique nodes,
which is a costly operation. This is intended to limit
the number of RAUW, and is very effective on the total
link-time of opt with ThinLTO, before:

  real 4m4.587s  user 15m3.401s  sys 0m23.616s

after:

  real 3m25.261s user 12m22.132s sys 0m24.152s

Reviewers: tejohnson, pcc

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28751

llvm-svn: 292420
2017-01-18 18:36:21 +00:00
Malcolm Parsons 17d266bc96 Remove unused lambda captures. NFC
llvm-svn: 291916
2017-01-13 17:12:16 +00:00
Benjamin Kramer 061f4a5fe6 Apply clang-tidy's performance-unnecessary-value-param to LLVM.
With some minor manual fixes for using function_ref instead of
std::function. No functional change intended.

llvm-svn: 291904
2017-01-13 14:39:03 +00:00
Mehdi Amini 7b0d145768 [ThinLTO] Fix lazy-loading of Metadata attachment, which left some Fwd ref behind
The change in r291362 was too agressive. We still need to flush at the
end of the block because function local metadata can introduce fwd
ref as well.
(Bootstrap with ThinLTO was broken)

llvm-svn: 291379
2017-01-08 00:44:45 +00:00
Mehdi Amini d5549f3dac [ThinLTO] Fix assertions on lazy-loading of Metadata TBAA attachments
Summary:
The issue happens with:

 %0 = ....., !tbaa !0
 %1 = ....., !tbaa !1

With !0 that references !1.

In this case when loading !0 we generates a temporary for the
operand !1. We now flush it immediately and trigger the load of
!1 before moving on. If we don't we get the temporary when
attaching to %1. This is usually not an issue except that we
eagerly try to update TBAA MDNodes, which is obviously not possible
if we only have a temporary.

Differential Revision: https://reviews.llvm.org/D28423

llvm-svn: 291362
2017-01-07 20:24:23 +00:00
Mehdi Amini 42ef199058 [Bitcode] Remove unused PlaceHolder parameter to lazyLoadModuleMetadataBlock()
llvm-svn: 291356
2017-01-07 18:31:38 +00:00
Teresa Johnson 6c475a7595 ThinLTO: add early "dead-stripping" on the Index
Summary:
Using the linker-supplied list of "preserved" symbols, we can compute
the list of "dead" symbols, i.e. the one that are not reachable from
a "preserved" symbol transitively on the reference graph.
Right now we are using this information to mark these functions as
non-eligible for import.

The impact is two folds:
- Reduction of compile time: we don't import these functions anywhere
  or import the function these symbols are calling.
- The limited number of import/export leads to better internalization.

Patch originally by Mehdi Amini.

Reviewers: mehdi_amini, pcc

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D23488

llvm-svn: 291177
2017-01-05 21:34:18 +00:00
Teresa Johnson 519465b993 [ThinLTO] Subsume all importing checks into a single flag
Summary:
This adds a new summary flag NotEligibleToImport that subsumes
several existing flags (NoRename, HasInlineAsmMaybeReferencingInternal
and IsNotViableToInline). It also subsumes the checking of references
on the summary that was being done during the thin link by
eligibleForImport() for each candidate. It is much more efficient to
do that checking once during the per-module summary build and record
it in the summary.

Reviewers: mehdi_amini

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28169

llvm-svn: 291108
2017-01-05 14:32:16 +00:00
Mehdi Amini 19ef4fad91 Use lazy-loading of Metadata in MetadataLoader when importing is enabled (NFC)
Summary:
This is a relatively simple scheme: we use the index emitted in the
bitcode to avoid loading all the global metadata. Instead we load
the index with their position in the bitcode so that we can load each
of them individually. Materializing the global metadata block in this
condition only triggers loading the named metadata, and the ones
referenced from there (transitively). When materializing a function,
metadata from the global block are loaded lazily as they are
referenced.

Two main current limitations are:

1) Global values other than functions are not materialized on demand,
so we need to eagerly load METADATA_GLOBAL_DECL_ATTACHMENT records
(and their transitive dependencies).
2) When we load a single metadata, we don't recurse on the operands,
instead we use a placeholder or a temporary metadata. Unfortunately
tepmorary nodes are very expensive. This is why we don't have it
always enabled and only for importing.

These two limitations can be lifted in a subsequent improvement if
needed.

With this change, the total link time of opt with ThinLTO and Debug
Info enabled is going down from 282s to 224s (~20%).

Reviewers: pcc, tejohnson, dexonsmith

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28113

llvm-svn: 291027
2017-01-04 22:54:33 +00:00
Mehdi Amini 867aad1359 Change BitstreamCursor::skipRecord to return the record code (NFC)
llvm-svn: 291026
2017-01-04 22:54:14 +00:00
David Blaikie 7ad9dc11db Reapply "Make BitCodeAbbrev ownership explicit using shared_ptr rather than IntrusiveRefCntPtr""
If this is a problem for anyone (shared_ptr is two pointers in size,
whereas IntrusiveRefCntPtr is 1 - and the ref count control block that
make_shared adds is probably larger than the one int in RefCountedBase)
I'd prefer to address this by adding a lower-overhead version of
shared_ptr (possibly refactoring IntrusiveRefCntPtr into such a thing)
to avoid the intrusiveness - this allows memory ownership to remain
orthogonal to types and at least to me, seems to make code easier to
understand (since no implicit ownership acquisition can happen).

This recommits 291006, reverted in r291007.

llvm-svn: 291016
2017-01-04 22:36:33 +00:00
David Blaikie 6e2207a134 Revert "Make BitCodeAbbrev ownership explicit using shared_ptr rather than IntrusiveRefCntPtr"
Breaks Clang's use of bitcode. Reverting until I have a fix to go with
it there.

This reverts commit r291006.

llvm-svn: 291007
2017-01-04 21:19:28 +00:00
David Blaikie daff78cd87 Make BitCodeAbbrev ownership explicit using shared_ptr rather than IntrusiveRefCntPtr
If this is a problem for anyone (shared_ptr is two pointers in size,
whereas IntrusiveRefCntPtr is 1 - and the ref count control block that
make_shared adds is probably larger than the one int in RefCountedBase)
I'd prefer to address this by adding a lower-overhead version of
shared_ptr (possibly refactoring IntrusiveRefCntPtr into such a thing)
to avoid the intrusiveness - this allows memory ownership to remain
orthogonal to types and at least to me, seems to make code easier to
understand (since no implicit ownership acquisition can happen).

llvm-svn: 291006
2017-01-04 21:13:35 +00:00
Teresa Johnson 5a8dba5bda [ThinLTO] Import type as decl only when non-null Identifier
As per post-commit review for r289993 (D27775), we can only safely
import a type as a decl if it has an Identifier, as the Name alone
is not enough to be unique across modules.

llvm-svn: 290915
2017-01-03 23:19:29 +00:00
Mehdi Amini 5022bb7238 Change Metadata Index emission in the bitcode to use 2x32 bits for the placeholder
The Bitstream reader and writer are limited to handle a "size_t" at
most, which means that we can't backpatch and read back a 64bits
value on 32 bits platform.

llvm-svn: 290693
2016-12-28 23:45:54 +00:00
Mehdi Amini e98f925834 Add an index for Module Metadata record in the bitcode
This index record the position for each metadata record in
the bitcode, so that the reader will be able to lazy-load
on demand each individual record.

We also make sure that every abbrev is emitted upfront so
that the block can be skipped while reading.

I don't plan to commit this before having the reader
counterpart, but I figured this can be reviewed mostly
independently.

Recommit r290684 (was reverted in r290686 because a test
was broken) after adding a threshold to avoid emitting
the index when unnecessary (little amount of metadata).
This optimization "hides" a limitation of the ability
to backpatch in the bitstream: we can only backpatch
safely when the position has been flushed. So if we emit
an index for one metadata, it is possible that (part of)
the offset placeholder hasn't been flushed and the backpatch
will fail.

Differential Revision: https://reviews.llvm.org/D28083

llvm-svn: 290690
2016-12-28 22:30:28 +00:00
Saleem Abdulrasool 2b59eca1f7 Revert "Add an index for Module Metadata record in the bitcode"
This reverts commit a0ca6ae2d38339e4ede0dfa588086fc23d87e836.  Revert at
Mehdi's request as it is breaking bots.

llvm-svn: 290686
2016-12-28 20:37:22 +00:00
Mehdi Amini 32ca148198 Add an index for Module Metadata record in the bitcode
Summary:
This index record the position for each metadata record in
the bitcode, so that the reader will be able to lazy-load
on demand each individual record.

We also make sure that every abbrev is emitted upfront so
that the block can be skipped while reading.

I don't plan to commit this before having the reader
counterpart, but I figured this can be reviewed mostly
independently.

Reviewers: pcc, tejohnson

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28083

llvm-svn: 290684
2016-12-28 19:44:19 +00:00
Amjad Aboud 7faeecc8f7 [DebugInfo] Added support for Checksum debug info feature.
Differential Revision: https://reviews.llvm.org/D27642

llvm-svn: 290514
2016-12-25 10:12:09 +00:00
Mehdi Amini 690952d15e MetadataLoader: replace the tracking of ForwardReferences and UnresolvedNodes with a set-based solution (NFC)
This makes it explicit what is the exact list to handle, and it
looks much more easy to manipulate and understand that the
previous custom tracking of min/max to express the range where
to look for.

Differential Revision: https://reviews.llvm.org/D28089

llvm-svn: 290507
2016-12-25 04:22:54 +00:00
Mehdi Amini 4f90ee0010 MetadataLoader: add an extra assertion in Placeholders flush (NFC)
We don't expect any forward reference at this point.

llvm-svn: 290506
2016-12-25 03:55:53 +00:00
Mehdi Amini 9f926f70c1 MetadataLoader: split the creation of a single metadata out of a Record into its own function (NFC)
This is pure code motion, will just make it more reusable when I'll
attempt to lazy-load Metadats on-demand.

llvm-svn: 290414
2016-12-23 03:59:18 +00:00
Mehdi Amini 37c178b6f5 MetadataLoader: Reinitialize MinFwdRef/MaxFwdRef after resolving cycles (NFC)
This put the Loader back in a consistent state.

llvm-svn: 290409
2016-12-23 02:20:12 +00:00
Mehdi Amini 5ae6170fc2 MetadataLoader: Add an assertion for the implicit invariant of PlaceHolder while loading Metadata (NFC)
llvm-svn: 290408
2016-12-23 02:20:09 +00:00
Mehdi Amini 70a9cd4cbe MetadataLoader: Make sure every member of MetadataLoader are initialized (NFC)
llvm-svn: 290407
2016-12-23 02:20:07 +00:00
Mehdi Amini ec68dd49bf MetadataLoader: Refactor "IsImporting" into the Pimpl for the MetadataLoader (NFC)
Keeping all the state together will make it easier to handle.

llvm-svn: 290406
2016-12-23 02:20:02 +00:00
Peter Collingbourne 704f814a5e Clear the PendingTypeTests vector after moving from it.
This is to put the vector into a well defined state. Apparently the state of a
vector after being moved from is valid but unspecified. Found with clang-tidy.

llvm-svn: 290298
2016-12-22 02:52:23 +00:00
Peter Collingbourne 1b4137a7f9 IR: Function summary representation for type tests.
Each function summary has an attached list of type identifier GUIDs. The
idea is that during the regular LTO phase we would match these GUIDs to type
identifiers defined by the regular LTO module and store the resolutions in
a top-level "type identifier summary" (which will be implemented separately).

Differential Revision: https://reviews.llvm.org/D27967

llvm-svn: 290280
2016-12-21 23:03:45 +00:00
Peter Collingbourne 0c30f089d5 IR: Eliminate non-determinism in the module summary analysis.
Also make the summary ref and call graph vectors immutable. This means
a smaller API surface and fewer places to audit for non-determinism.

Differential Revision: https://reviews.llvm.org/D27875

llvm-svn: 290200
2016-12-20 21:12:28 +00:00
Adrian Prantl bceaaa9643 [IR] Remove the DIExpression field from DIGlobalVariable.
This patch implements PR31013 by introducing a
DIGlobalVariableExpression that holds a pair of DIGlobalVariable and
DIExpression.

Currently, DIGlobalVariables holds a DIExpression. This is not the
best way to model this:

(1) The DIGlobalVariable should describe the source level variable,
    not how to get to its location.

(2) It makes it unsafe/hard to update the expressions when we call
    replaceExpression on the DIGLobalVariable.

(3) It makes it impossible to represent a global variable that is in
    more than one location (e.g., a variable with multiple
    DW_OP_LLVM_fragment-s).  We also moved away from attaching the
    DIExpression to DILocalVariable for the same reasons.

This reapplies r289902 with additional testcase upgrades and a change
to the Bitcode record for DIGlobalVariable, that makes upgrading the
old format unambiguous also for variables without DIExpressions.

<rdar://problem/29250149>
https://llvm.org/bugs/show_bug.cgi?id=31013
Differential Revision: https://reviews.llvm.org/D26769

llvm-svn: 290153
2016-12-20 02:09:43 +00:00
Teresa Johnson a61f5e3796 [ThinLTO] Import composite types as declarations
Summary:
When reading the metadata bitcode, create a type declaration when
possible for composite types when we are importing. Doing this in
the bitcode reader saves memory. Also it works naturally in the case
when the type ODR map contains a definition for the same composite type
because it was used in the importing module (buildODRType will
automatically use the existing definition and not create a type
declaration).

For Chromium built with -g2, this reduces the aggregate size of the
generated native object files by 66% (from 31G to 10G). It reduced
the time through the ThinLTO link and backend phases by about 20% on
my machine.

Reviewers: mehdi_amini, dblaikie, aprantl

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D27775

llvm-svn: 289993
2016-12-16 21:25:01 +00:00
Adrian Prantl 73ec065604 Revert "[IR] Remove the DIExpression field from DIGlobalVariable."
This reverts commit 289920 (again).
I forgot to implement a Bitcode upgrade for the case where a DIGlobalVariable
has not DIExpression. Unfortunately it is not possible to safely upgrade
these variables without adding a flag to the bitcode record indicating which
version they are.
My plan of record is to roll the planned follow-up patch that adds a
unit: field to DIGlobalVariable into this patch before recomitting.
This way we only need one Bitcode upgrade for both changes (with a
version flag in the bitcode record to safely distinguish the record
formats).

Sorry for the churn!

llvm-svn: 289982
2016-12-16 19:39:01 +00:00
Mehdi Amini 8662305bae Strip invalid TBAA when reading bitcode
This ensures backward compatibility on bitcode loading.

Differential Revision: https://reviews.llvm.org/D27839

llvm-svn: 289977
2016-12-16 19:16:29 +00:00
Adrian Prantl 74a835cda0 [IR] Remove the DIExpression field from DIGlobalVariable.
This patch implements PR31013 by introducing a
DIGlobalVariableExpression that holds a pair of DIGlobalVariable and
DIExpression.

Currently, DIGlobalVariables holds a DIExpression. This is not the
best way to model this:

(1) The DIGlobalVariable should describe the source level variable,
    not how to get to its location.

(2) It makes it unsafe/hard to update the expressions when we call
    replaceExpression on the DIGLobalVariable.

(3) It makes it impossible to represent a global variable that is in
    more than one location (e.g., a variable with multiple
    DW_OP_LLVM_fragment-s).  We also moved away from attaching the
    DIExpression to DILocalVariable for the same reasons.

This reapplies r289902 with additional testcase upgrades.

<rdar://problem/29250149>
https://llvm.org/bugs/show_bug.cgi?id=31013
Differential Revision: https://reviews.llvm.org/D26769

llvm-svn: 289920
2016-12-16 04:25:54 +00:00