[NFC][DebugInfo] Use iterator moveBefore at many call-sites (#123583)

As part of the "RemoveDIs" project, BasicBlock::iterator now carries a
debug-info bit that's needed when getFirstNonPHI and sim

[NFC][DebugInfo] Use iterator moveBefore at many call-sites (#123583)

As part of the "RemoveDIs" project, BasicBlock::iterator now carries a
debug-info bit that's needed when getFirstNonPHI and similar feed into
instruction insertion positions. Call-sites where that's necessary were
updated a year ago; but to ensure some type safety however, we'd like to
have all calls to moveBefore use iterators.

This patch adds a (guaranteed dereferenceable) iterator-taking
moveBefore, and changes a bunch of call-sites where it's obviously safe
to change to use it by just calling getIterator() on an instruction
pointer. A follow-up patch will contain less-obviously-safe changes.

We'll eventually deprecate and remove the instruction-pointer
insertBefore, but not before adding concise documentation of what
considerations are needed (very few).

show more ...

[DebugInfo][RemoveDIs] Use iterator-insertion in unittests and fuzzer (#102015)

These are the final few places in LLVM that use instruction pointers to
insert instructions -- use iterators instead,

[DebugInfo][RemoveDIs] Use iterator-insertion in unittests and fuzzer (#102015)

These are the final few places in LLVM that use instruction pointers to
insert instructions -- use iterators instead, which is needed for
debug-info correctness in the future. Most of this is a gentle
scattering of getIterator calls or not deref-then-addrofing iterators.
libfuzzer does require a storage change to keep built instruction
positions in a container though. The unit-test changes are very
straightforwards.

This leaves us in a position where libfuzzer can't fuzz on either of
debug-info records, however I don't believe that fuzzing of debug-info
is in scope for the library.

show more ...

[CGSCC] Remove CGSCCUpdateResult::InvalidatedRefSCCs (#98213)

The RefSCC that a function marked dead is in may still contain valid
SCCs that we want to visit. We rely on InvalidatedSCCs to skip SCC

[CGSCC] Remove CGSCCUpdateResult::InvalidatedRefSCCs (#98213)

The RefSCC that a function marked dead is in may still contain valid
SCCs that we want to visit. We rely on InvalidatedSCCs to skip SCCs
containing dead functions.

The addition of RefSCCs in CallGraphUpdater to InvalidatedRefSCCs was
causing asserts as reported in #94815. Fix some more CallGraphUpdater
function deletion methods as well.

show more ...

[IR] Don't include PassInstrumentation.h in PassManager.h (NFC) (#96219)

Move PassInstrumentationAnalysis into PassInstrumentation.h and stop
including it in PassManager.h (effectively inverting th

[IR] Don't include PassInstrumentation.h in PassManager.h (NFC) (#96219)

Move PassInstrumentationAnalysis into PassInstrumentation.h and stop
including it in PassManager.h (effectively inverting the direction of
the dependency).

Most places using PassManager are not interested in PassInstrumentation,
and we no longer have any uses of it in PassManager.h itself (only in
PassManagerImpl.h).

show more ...

[CGSCC] Fix compile time blowup with large RefSCCs (#94815)

In some modules, e.g. Kotlin-generated IR, we end up with a huge RefSCC
and the call graph updates done as a result of the inliner take a

[CGSCC] Fix compile time blowup with large RefSCCs (#94815)

In some modules, e.g. Kotlin-generated IR, we end up with a huge RefSCC
and the call graph updates done as a result of the inliner take a long
time. This is due to RefSCC::removeInternalRefEdges() getting called
many times, each time removing one function from the RefSCC, but each
call to removeInternalRefEdges() is proportional to the size of the
RefSCC.

There are two places that call removeInternalRefEdges(), in
updateCGAndAnalysisManagerForPass() and
LazyCallGraph::removeDeadFunction().

1) Since LazyCallGraph can deal with spurious (edges that exist in the
graph but not in the IR) ref edges, we can simply not call
removeInternalRefEdges() in updateCGAndAnalysisManagerForPass().

2) LazyCallGraph::removeDeadFunction() still ends up taking the brunt of
compile time with the above change for the original reason. So instead
we batch all the dead function removals so we can call
removeInternalRefEdges() just once. This requires some changes to
callers of removeDeadFunction() to not actually erase the function from
the module, but defer it to when we batch delete dead functions at the
end of the CGSCC run, leaving the function body as "unreachable" in the
meantime. We still need to ensure that call edges are accurate. I had
also tried deleting dead functions after visiting a RefSCC, but deleting
them all at once at the end was simpler.

Many test changes are due to not performing unnecessary revisits of an
SCC (the CGSCC infrastructure deems ref edge refinements as unimportant
when it comes to revisiting SCCs, although that seems to not be
consistently true given these changes) because we don't remove some ref
edges. Specifically for devirt-invalidated.ll this seems to expose an
inlining order issue with the inliner. Probably unimportant for this
type of intentionally weird call graph.

Compile time:
https://llvm-compile-time-tracker.com/compare.php?from=6f2c61071c274a1b5e212e6ad4114641ec7c7fc3&to=b08c90d05e290dd065755ea776ceaf1420680224&stat=instructions:u

show more ...

[CGSCCPassManagerTest] Replace no-op bitcast to create constant using the old function (#72460)

With opaque pointers enabled, the existing ptr-to-ptr bitcast is a no-op
and no longer creates a cons

[CGSCCPassManagerTest] Replace no-op bitcast to create constant using the old function (#72460)

With opaque pointers enabled, the existing ptr-to-ptr bitcast is a no-op
and no longer creates a constant that references the old function.

Replace the no-op bitcast with code that creates a constant that
references the old function. The test now fails if the 1 new line of
code added to `CallGraphUpdater::replaceFunctionWith()` in
cb0ecc5c33bd56a3eed0fa30ac787accec45d637 is removed (test passes if kept
intact).

---------

Co-authored-by: Nikita Popov <github@npopov.com>

show more ...

[NFC] Remove Type::getInt8PtrTy (#71029)

Replace this with PointerType::getUnqual().
Followup to the opaque pointer transition. Fixes an in-code TODO item.

[NFC] Add #include for constants

Revert "[NPM] Do not run function simplification pipeline unnecessarily"

This reverts commit 97ab068034161fb35e5c9a7b293bf1e569cf077b.

Depends on D100917, which is to be reverted.

[NewPM] Hide pass manager debug logging behind -debug-pass-manager-verbose

Printing pass manager invocations is fairly verbose and not super
useful.

This allows us to remove DebugLogging from pass

[NewPM] Hide pass manager debug logging behind -debug-pass-manager-verbose

Printing pass manager invocations is fairly verbose and not super
useful.

This allows us to remove DebugLogging from pass managers and PassBuilder
since all logging (aside from analysis managers) goes through
instrumentation now.

This has the downside of never being able to print the top level pass
manager via instrumentation, but that seems like a minor downside.

Reviewed By: ychen

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

show more ...

[NewPM] Move analysis invalidation/clearing logging to instrumentation

We're trying to move DebugLogging into instrumentation, rather than
being part of PassManagers/AnalysisManagers.

Reviewed By:

[NewPM] Move analysis invalidation/clearing logging to instrumentation

We're trying to move DebugLogging into instrumentation, rather than
being part of PassManagers/AnalysisManagers.

Reviewed By: ychen

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

show more ...

[NPM] Do not run function simplification pipeline unnecessarily

The CGSCC pass manager interplay with the FunctionAnalysisManagerCGSCCProxy is 'special' in the sense that the former will rerun the l

[NPM] Do not run function simplification pipeline unnecessarily

The CGSCC pass manager interplay with the FunctionAnalysisManagerCGSCCProxy is 'special' in the sense that the former will rerun the latter if there are changes to a SCC structure; that being said, some of the functions in the SCC may be unchanged. In that case, the function simplification pipeline will be re-run, which impacts compile time[1].

This patch allows the function simplification pipeline be skipped if it was already run and the function was not modified since.

The behavior is currently disabled by default. This is because, currently, the rerunning of the function simplification pipeline on an unchanged function may still result in changes. The patch simplifies investigating and fixing those cases where repeated function pass runs do actually positively impact code quality, while offering an easy workaround for those impacted negatively by compile time regressions, and not impacting mainline scenarios.

[1] A [[ http://llvm-compile-time-tracker.com/compare.php?from=eb37d3546cd0c6e67798496634c45e501f7806f1&to=ac722d1190dc7bbdd17e977ef7ec95e69eefc91e&stat=instructions | compile time tracker ]] run with the option enabled.

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

show more ...

Reapply "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes"

This reverts commit 11b70b9e3a7458b5b78c30020b56e8ca563a4801.

The bot failure was due to ArgumentPro

Reapply "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes"

This reverts commit 11b70b9e3a7458b5b78c30020b56e8ca563a4801.

The bot failure was due to ArgumentPromotion deleting functions
without deleting their analyses. This was separately fixed in 4b1c807.

show more ...

Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes"

This reverts commit 5eaeb0fa67e57391f5584a3f67fdb131e93afda6.

It appears there are analyses that assu

Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes"

This reverts commit 5eaeb0fa67e57391f5584a3f67fdb131e93afda6.

It appears there are analyses that assume clearing - example:
https://lab.llvm.org/buildbot#builders/36/builds/5964

show more ...

[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes

Check with the analysis result by calling invalidate instead of clear on
the analysis manager.

Differential Re

[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes

Check with the analysis result by calling invalidate instead of clear on
the analysis manager.

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

show more ...

[CGSCC][Coroutine][NewPM] Properly support function splitting/outlining

Previously when trying to support CoroSplit's function splitting, we
added in a hack that simply added the new function's node

[CGSCC][Coroutine][NewPM] Properly support function splitting/outlining

Previously when trying to support CoroSplit's function splitting, we
added in a hack that simply added the new function's node into the
original function's SCC (https://reviews.llvm.org/D87798). This is
incorrect since it might be in its own SCC.

Now, more similar to the previous design, we have callers explicitly
notify the LazyCallGraph that a function has been split out from another
one.

In order to properly support CoroSplit, there are two ways functions can
be split out.

One is the normal expected "outlining" of one function into a new one.
The new function may only contain references to other functions that the
original did. The original function must reference the new function. The
new function may reference the original function, which can result in
the new function being in the same SCC as the original function. The
weird case is when the original function indirectly references the new
function, but the new function directly calls the original function,
resulting in the new SCC being a parent of the original function's SCC.
This form of function splitting works with CoroSplit's Switch ABI.

The second way of splitting is more specific to CoroSplit. CoroSplit's
Retcon and Async ABIs split the original function into multiple
functions that all reference each other and are referenced by the
original function. In order to keep the LazyCallGraph in a valid state,
all new functions must be processed together, else some nodes won't be
populated. To keep things simple, this only supports the case where all
new edges are ref edges, and every new function references every other
new function. There can be a reference back from any new function to the
original function, putting all functions in the same RefSCC.

This also adds asserts that all nodes in a (Ref)SCC can reach all other
nodes to prevent future incorrect hacks.

The original hacks in https://reviews.llvm.org/D87798 are no longer
necessary since all new functions should have been registered before
calling updateCGAndAnalysisManagerForPass.

This fixes all coroutine tests when opt's -enable-new-pm is true by
default. This also fixes PR48190, which was likely due to the previous
hack breaking SCC invariants.

Reviewed By: rnk

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

show more ...

[test] Factor out creation of copy of SCC Nodes into function

Reviewed By: rnk

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

[test] Cleanup some CGSCCPassManager tests

Don't iterate over SCC as we potentially modify it.
Verify module (and fix some broken ones).
Only run pass once and make sure that it's actually run.
Rena

[test] Cleanup some CGSCCPassManager tests

Don't iterate over SCC as we potentially modify it.
Verify module (and fix some broken ones).
Only run pass once and make sure that it's actually run.
Rename tests to just end in a number since I'm planning on adding a
bunch more which won't have good individual names. Instead, add comments
on the transformations that each test does.

Reviewed By: rnk

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

show more ...

[CGSCC][Inliner] Handle new non-trivial edges in updateCGAndAnalysisManagerForPass

Previously the inliner did a bit of a hack by adding ref edges for all
new edges introduced by performing an inline

[CGSCC][Inliner] Handle new non-trivial edges in updateCGAndAnalysisManagerForPass

Previously the inliner did a bit of a hack by adding ref edges for all
new edges introduced by performing an inline before calling
updateCGAndAnalysisManagerForPass(). This was because
updateCGAndAnalysisManagerForPass() didn't handle new non-trivial call
edges.

This adds handling of non-trivial call edges to
updateCGAndAnalysisManagerForPass(). The inliner called
updateCGAndAnalysisManagerForFunctionPass() since it was handling adding
newly introduced edges (so updateCGAndAnalysisManagerForPass() would
only have to handle promotion), but now it needs to call
updateCGAndAnalysisManagerForCGSCCPass() since
updateCGAndAnalysisManagerForPass() is now handling the new call edges
and function passes cannot add new edges.

We follow the previous path of adding trivial ref edges then letting promotion
handle changing the ref edges to call edges and the CGSCC updates. So
this still does not allow adding call edges that result in an addition
of a non-trivial ref edge.

This is in preparation for better detecting devirtualization. Previously
since the inliner itself would add ref edges,
updateCGAndAnalysisManagerForPass() would think that promotion and thus
devirtualization had happened after any sort of inlining.

Reviewed By: asbirlea

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

show more ...

[LazyCallGraph] Build SCCs of the reference graph in order

```
// The legacy PM CGPassManager discovers SCCs this way:
for function in the source order
tarjanSCC(function)

// While the new PM CGS

[LazyCallGraph] Build SCCs of the reference graph in order

```
// The legacy PM CGPassManager discovers SCCs this way:
for function in the source order
tarjanSCC(function)

// While the new PM CGSCCPassManager does:
for function in the reversed source order [1]
discover a reference graph SCC
build call graph SCCs inside the reference graph SCC
```

In the common cases, reference graph ~= call graph, the new PM order is
undesired because for `a | b | c` (3 independent functions), the new PM will
process them in the reversed order: c, b, a. If `a <-> b <-> c`, we can see
that `-print-after-all` will report the sole SCC as `scc: (c, b, a)`.

This patch corrects the iteration order. The discovered SCC order will match
the legacy PM in the common cases.

For some tests (`Transforms/Inline/cgscc-*.ll` and
`unittests/Analysis/CGSCCPassManagerTest.cpp`), the behaviors are dependent on
the SCC discovery order and there are too many check lines for the particular
order. This patch simply reverses the function order to avoid changing too many
check lines.

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

show more ...

[NewPM][CGSCC] Handle newly added functions in updateCGAndAnalysisManagerForPass

This seems to fit the CGSCC updates model better than calling
addNewFunctionInto{Ref,}SCC() on newly created/outlined

[NewPM][CGSCC] Handle newly added functions in updateCGAndAnalysisManagerForPass

This seems to fit the CGSCC updates model better than calling
addNewFunctionInto{Ref,}SCC() on newly created/outlined functions.
Now addNewFunctionInto{Ref,}SCC() are no longer necessary.

However, this doesn't work on newly outlined functions that aren't
referenced by the original function. e.g. if a() was outlined into b()
and c(), but c() is only referenced by b() and not by a(), this will
trigger an assert.

This also fixes an issue I was seeing with newly created functions not
having passes run on them.

Ran check-llvm with expensive checks.

Reviewed By: asbirlea

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

show more ...

[CGSCC][NewPM] Fix adding mutually recursive new functions

When adding a new function via addNewFunctionIntoRefSCC(), it creates a
new node and immediately populates the edges. Since populateSlow()

[CGSCC][NewPM] Fix adding mutually recursive new functions

When adding a new function via addNewFunctionIntoRefSCC(), it creates a
new node and immediately populates the edges. Since populateSlow() calls
G->get() on all referenced functions, it will create a node (but not
populate it) for functions that haven't yet been added. If we add two
mutually recursive functions, the assert that the node should never have
been created will fire when the second function is added. So here we
remove that assert since the node may have already been created (but not
yet populated).

createNode() is only called from addNewFunctionInto{,Ref}SCC().

https://bugs.llvm.org/show_bug.cgi?id=47502

Reviewed By: jdoerfert

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

show more ...

[NewPM] Avoid redundant CGSCC run for updated SCC

Summary:
When an SCC got split due to inlining, we have two mechanisms for reprocessing the updated SCC, first is UR.UpdatedC
that repeatedly rerun

[NewPM] Avoid redundant CGSCC run for updated SCC

Summary:
When an SCC got split due to inlining, we have two mechanisms for reprocessing the updated SCC, first is UR.UpdatedC
that repeatedly rerun the new, current SCC; second is a worklist for all newly split SCCs. We can avoid rerun of
the same SCC when the SCC is set to be processed by both mechanisms *back to back*. In pathological cases, such redundant
rerun could cause exponential size growth due to inlining along cycles, even when there's no SCC mutation and hence
convergence is not a problem.

Note that it's ok to have SCC updated and rerun immediately, and also in the work list if we have actually moved an SCC
to be topologically "below" the current one due to merging. In that case, we will need to revisit the current SCC after
those moved SCCs. For that reason, the redundant avoidance here only targets back to back rerun of the same SCC - the
case described by the now removed FIXME comment.

Reviewers: chandlerc, wmi

Subscribers: llvm-commits, hoy

Tags: #llvm

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

show more ...

[NewPassManager] Add assertions when getting statefull cached analysis.

Summary:
Analyses that are statefull should not be retrieved through a proxy from
an outer IR unit, as these analyses are only

[NewPassManager] Add assertions when getting statefull cached analysis.

Summary:
Analyses that are statefull should not be retrieved through a proxy from
an outer IR unit, as these analyses are only invalidated at the end of
the inner IR unit manager.
This patch disallows getting the outer manager and provides an API to
get a cached analysis through the proxy. If the analysis is not
stateless, the call to getCachedResult will assert.

Reviewers: chandlerc

Subscribers: mehdi_amini, eraman, hiraditya, zzheng, llvm-commits

Tags: #llvm

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

show more ...

[CallSite removal][TargetLibraryInfo] Replace ImmutableCallSite with CallBase in one of the getLibFunc signatures. NFC

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