[IPO] Use range-based for loops (NFC)

[FunctionAttrs] Add missing pass dependency

This pass depends on AAResults. This fixes the ocaml IPO binding
tests.

[RPOFuncAttrs] Fix norecurse detection

We wanted to check if all uses of the function are direct calls, but the
code didn't account for passing the function as a parameter.

Reviewed By: nikic

Diff

[RPOFuncAttrs] Fix norecurse detection

We wanted to check if all uses of the function are direct calls, but the
code didn't account for passing the function as a parameter.

Reviewed By: nikic

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

show more ...

Cleanup includes: Transforms/IPO

Preprocessor output diff: -238205 lines
Discourse thread: https://discourse.llvm.org/t/include-what-you-use-include-cleanup
Differential Revision: https://reviews.ll

Cleanup includes: Transforms/IPO

Preprocessor output diff: -238205 lines
Discourse thread: https://discourse.llvm.org/t/include-what-you-use-include-cleanup
Differential Revision: https://reviews.llvm.org/D122183

show more ...

[FunctionAttrs] Infer argmemonly .

This patch adds initial argmemonly inference, by checking the underlying
objects of locations returned by MemoryLocation.

I think this should cover most cases, ex

[FunctionAttrs] Infer argmemonly .

This patch adds initial argmemonly inference, by checking the underlying
objects of locations returned by MemoryLocation.

I think this should cover most cases, except function calls to other
argmemonly functions.

I'm not sure if there's a reason why we don't infer those yet.

Additional argmemonly can improve codegen in some cases. It also makes
it easier to come up with a C reproducer for 7662d1687b09 (already fixed,
but I'm trying to see if C/C++ fuzzing could help to uncover similar
issues.)

Compile-time impact:
NewPM-O3: +0.01%
NewPM-ReleaseThinLTO: +0.03%
NewPM-ReleaseLTO+g: +0.05%

https://llvm-compile-time-tracker.com/compare.php?from=067c035012fc061ad6378458774ac2df117283c6&to=fe209d4aab5b593bd62d18c0876732ddcca1614d&stat=instructions

Reviewed By: nikic

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

show more ...

[FunctionAttrs] Replace MemoryAccessKind with FMRB.

Update FunctionAttrs to use FunctionModRefBehavior instead
MemoryAccessKind.

This allows for adding support for inferring argmemonly and others,

[FunctionAttrs] Replace MemoryAccessKind with FMRB.

Update FunctionAttrs to use FunctionModRefBehavior instead
MemoryAccessKind.

This allows for adding support for inferring argmemonly and others,
see D121415.

Reviewed By: nikic

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

show more ...

[FunctionAttrs] Rename addReadAttrs -> addMemoryAttrs.

The addReadAttrs name is out of date, as the function also adds
the writeonly attribute. addMemoryAttrs is more accurate.

[funcattrs] check reachability to improve noreturn

There was a fixme in the code pertaining to attributing functions as
noreturn. By using reachability, if none of the blocks that are
reachable fro

[funcattrs] check reachability to improve noreturn

There was a fixme in the code pertaining to attributing functions as
noreturn. By using reachability, if none of the blocks that are
reachable from the entry return, then the function is noreturn.

Previously, the code only checked if any blocks returned. If they're
unreachable, then they don't matter.

This improves codegen for the Linux kernel.

Fixes: https://github.com/ClangBuiltLinux/linux/issues/1563

Reviewed By: nikic

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

show more ...

Rename doesNotReadMemory to onlyWritesMemory globally [NFC]

The naming has come up as a source of confusion in several recent reviews. onlyWritesMemory is consist with onlyReadsMemory which we use

Rename doesNotReadMemory to onlyWritesMemory globally [NFC]

The naming has come up as a source of confusion in several recent reviews. onlyWritesMemory is consist with onlyReadsMemory which we use for the corresponding readonly case as well.

show more ...

[funcattrs] Infer writeonly argument attribute [part 2]

This builds on the code from D114963, and extends it to handle calls both direct and indirect. With the revised code structure (from series of

[funcattrs] Infer writeonly argument attribute [part 2]

This builds on the code from D114963, and extends it to handle calls both direct and indirect. With the revised code structure (from series of previously landed NFCs), this is pretty straight forward.

One thing to note is that we can not infer writeonly for arguments which might be captured. If the pointer can be read back by the caller, and then read through, we have no way to track that. This is the same restriction we have for readonly, except that we get no mileage out of the "callee can be readonly" exception since a writeonly param on a readonly function is either a) readnone or b) UB. This means we can't actually infer much unless nocapture has already been inferred.

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

show more ...

Introduce the AttributeMask class

This class is solely used as a lightweight and clean way to build a set of
attributes to be removed from an AttrBuilder. Previously AttrBuilder was used
both for bu

Introduce the AttributeMask class

This class is solely used as a lightweight and clean way to build a set of
attributes to be removed from an AttrBuilder. Previously AttrBuilder was used
both for building and removing, which introduced odd situation like creation of
Attribute with dummy value because the only relevant part was the attribute
kind.

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

show more ...

[funcattrs] Use callsite param attributes from indirect calls when inferring access attributes

Arguments to an indirect call is by definition outside the SCC, but there's no reason we can't use loca

[funcattrs] Use callsite param attributes from indirect calls when inferring access attributes

Arguments to an indirect call is by definition outside the SCC, but there's no reason we can't use locally defined facts on the call site. This also has the nice effect of further simplifying the code.

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

show more ...

[funcattrs] Infer access attributes for vararg arguments

This change allows us to infer access attributes (readnone, readonly) on arguments passed to vararg functions. Since there isn't a formal arg

[funcattrs] Infer access attributes for vararg arguments

This change allows us to infer access attributes (readnone, readonly) on arguments passed to vararg functions. Since there isn't a formal argument corresponding to the parameter, they'll never be considered part of the speculative SCC, but they can still benefit from attributes on the call site or the callee function.

The main motivation here is just to simplify the code, and remove some special casing. Previously, an indirect vararg call could return more precise results than an direct vararg call which is just weird.

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

show more ...

[funcattrs] Fix incorrect readnone/readonly inference on captured arguments

This fixes a bug where we would infer readnone/readonly for a function which passed a value to a function which could capt

[funcattrs] Fix incorrect readnone/readonly inference on captured arguments

This fixes a bug where we would infer readnone/readonly for a function which passed a value to a function which could capture it. With the value captured in memory, the function could reload the value from memory after the call, and write to it. Inferring the argument as readnone or readonly is unsound.

@jdoerfert apparently noticed this about two years ago, and tests were checked in with 76467c4, but the issue appears to have never gotten fixed.

Since this seems like this issue should break everything, let me explain why the case is actually fairly narrow. The main inference loop over the argument SCCs only analyzes nocapture arguments. As such, we can only hit this when construction the partial SCCs. Due to that restriction, we can only hit this when we have either a) a function declaration with a manually annotated argument, or b) an immediately self recursive call.

It's also worth highlighting that we do have cases we can infer readonly/readnone on a capturing argument validly. The easiest example is a function which simply returns its argument without ever accessing it.

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

show more ...

[funcattrs] Use early return to clarify code in determinePointerAccessAttrs [NFC]

Instead of having the speculative path be the untaken path in the branch, explicitly have it return. This does requ

[funcattrs] Use early return to clarify code in determinePointerAccessAttrs [NFC]

Instead of having the speculative path be the untaken path in the branch, explicitly have it return. This does require tail duplicating one call, but the resulting code is shorter and easier to understand. Also rewrite the condition using appropriate accessors.

show more ...

[funcattrs] Use getDataOperandNo where appropriate [NFC]

We'd manually duplicated the same logic and assertions; we can use the utility instead.

[funcattrs] Consistently treat calling a function pointer as a non-capturing read

We were being wildly inconsistent about what memory access was implied by an indirect function call. Depending on th

[funcattrs] Consistently treat calling a function pointer as a non-capturing read

We were being wildly inconsistent about what memory access was implied by an indirect function call. Depending on the call site attributes, you could get anything from a read, to unknown, to none at all. (The last was a miscompile.)

We were also always traversing the uses of a readonly indirect call. This is entirely unneeded as the indirect call does not capture. The callee might capture itself internally, but that has no implications for this caller. (See the nice explanation in the CaptureTracking comments if that case is confusing.)

Note that elsewhere in the same file, we were correctly computing the nocapture attribute for indirect calls. The changed case only resulted in conservatism when computing memory attributes if say the return value was written to.

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

show more ...

[funcattrs] Fix a bug in recently introduced writeonly argument inference

This fixes a bug in 740057d. There's two ways to describe the issue:
* One caller hasn't yet proven nocapture on the argume

[funcattrs] Fix a bug in recently introduced writeonly argument inference

This fixes a bug in 740057d. There's two ways to describe the issue:
* One caller hasn't yet proven nocapture on the argument. Given that, the inference routine is responsible for bailing out on a potential capture.
* Even if we know the argument is nocapture, the access inference needs to traverse the exact set of users the capture tracking would (or exit conservatively). Even if capture tracking can prove a store is non-capturing (e.g. to a local alloc which doesn't escape), we still need to track the copy of the pointer to see if it's later reloaded and accessed again.

Note that all the test changes except the newly added ones appear to be false negatives. That is, cases where we could prove writeonly, but the current code isn't strong enough. That's why I didn't spot this originally.

show more ...

[funcattrs] Infer writeonly argument attribute

This change extends the current logic for inferring readonly and readnone argument attributes to also infer writeonly.

This change is deliberately min

[funcattrs] Infer writeonly argument attribute

This change extends the current logic for inferring readonly and readnone argument attributes to also infer writeonly.

This change is deliberately minimal; there's a couple of areas for follow up.
* I left out all call handling and thus any benefit from the SCC walk. When examining the test changes, I realized the existing code is imprecise, and am going to fix that in it's own revision before adding in the writeonly handling. (Mostly because updating the tests is hard when I, the human, can't figure out whether the result is correct.)
* I left out handling for storing a value (as opposed to storing to a pointer). This should benefit readonly/readnone as well, and applies to a bunch of other instructions. Seemed worth having as a separate review.

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

show more ...

[NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

Previously, any change in any function in an SCC would cause all
analyses for all functions

[NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

Previously, any change in any function in an SCC would cause all
analyses for all functions in the SCC to be invalidated. With this
change, we now manually invalidate analyses for functions we modify,
then let the pass manager know that all function analyses should be
preserved since we've already handled function analysis invalidation.

So far this only touches the inliner, argpromotion, function-attrs, and
updateCGAndAnalysisManager(), since they are the most used.

This is part of an effort to investigate running the function
simplification pipeline less on functions we visit multiple times in the
inliner pipeline.

However, this causes major memory regressions especially on larger IR.
To counteract this, turn on the option to eagerly invalidate function
analyses. This invalidates analyses on functions immediately after
they're processed in a module or scc to function adaptor for specific
parts of the pipeline.

Within an SCC, if a pass only modifies one function, other functions in
the SCC do not have their analyses invalidated, so in later function
passes in the SCC pass manager the analyses may still be cached. It is
only after the function passes that the eager invalidation takes effect.
For the default pipelines this makes sense because the inliner pipeline
runs the function simplification pipeline after all other SCC passes
(except CoroSplit which doesn't request any analyses).

Overall this has mostly positive effects on compile time and positive effects on memory usage.
https://llvm-compile-time-tracker.com/compare.php?from=7f627596977624730f9298a1b69883af1555765e&to=39e824e0d3ca8a517502f13032dfa67304841c90&stat=instructions
https://llvm-compile-time-tracker.com/compare.php?from=7f627596977624730f9298a1b69883af1555765e&to=39e824e0d3ca8a517502f13032dfa67304841c90&stat=max-rss

D113196 shows that we slightly regressed compile times in exchange for
some memory improvements when turning on eager invalidation. D100917
shows that we slightly improved compile times in exchange for major
memory regressions in some cases when invalidating less in SCC passes.
Turning these on at the same time keeps the memory improvements while
keeping compile times neutral/slightly positive.

Reviewed By: asbirlea, nikic

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

show more ...

[llvm] Use range-based for loops with instructions (NFC)

[llvm] Use range-based for loops with CallBase::args (NFC)

[NFC][FuncAttrs] Keep track of modified functions

This is in preparation for only invalidating analyses on changed
functions.

Reviewed By: asbirlea

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

[NFC][FuncAttrs] Keep track of modified functions

This is in preparation for only invalidating analyses on changed
functions.

Reviewed By: asbirlea

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

show more ...

Coroutines: don't infer function attrs before lowering

Coroutines have weird semantics that don't quite match normal LLVM functions,
so trying to infer even simple attributes based on thier contents

Coroutines: don't infer function attrs before lowering

Coroutines have weird semantics that don't quite match normal LLVM functions,
so trying to infer even simple attributes based on thier contents can go wrong.

show more ...

[Transforms] Use {DenseSet,SetVector,SmallPtrSet}::contains (NFC)