Revert "[MemDep][NFCI] Remove redundant dyn_cast, replace with cast"

This reverts commit 180d3f251d1ad5473705d3f00e6d426b5f8162e6. This commit is simply wrong. IsLoad is set within the same file b

Revert "[MemDep][NFCI] Remove redundant dyn_cast, replace with cast"

This reverts commit 180d3f251d1ad5473705d3f00e6d426b5f8162e6. This commit is simply wrong. IsLoad is set within the same file based on modref state, not whether the instruction is a LoadInst.

This went uncaught because cast<Ty>(X) has been broken. See https://discourse.llvm.org/t/cast-x-is-broken-implications-and-proposal-to-address/63033 for context.

show more ...

[NFC][MemDep] Remove unnecessary Worklist.clear

This execution path leads to return 'false' where the Worklist
will be deallocated anyways. No need to clear it separately.

[MemDep][NFC] Remove duplicating check in `if` and `else` branch

Same check is done whether the condition is true or false. Just hoist
it out of conditional.

[MemDep][NFCI] Remove redundant dyn_cast, replace with cast

When `IsLoad` is `true`, we don't need to check if the instruction
is actually a load with dyn_cast. Saves some petty amount of CT.

Cleanup includes: LLVMAnalysis

Number of lines output by preprocessor:
before: 1065940348
after: 1065307662

Discourse thread: https://discourse.llvm.org/t/include-what-you-use-include-cleanup
Diff

Cleanup includes: LLVMAnalysis

Number of lines output by preprocessor:
before: 1065940348
after: 1065307662

Discourse thread: https://discourse.llvm.org/t/include-what-you-use-include-cleanup
Differential Revision: https://reviews.llvm.org/D120659

show more ...

[MemoryDependency] Simplfy re-ordering condition. Cleanup. NFC.

Make the reading of condition for restricting re-ordering simpler.

Reviewers: reames
Reviewed By: reames
Subscribers: llvm-commits
Di

[MemoryDependency] Simplfy re-ordering condition. Cleanup. NFC.

Make the reading of condition for restricting re-ordering simpler.

Reviewers: reames
Reviewed By: reames
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D120005

show more ...

[MemoryDependency] Relax the re-ordering of atomic store and unordered load/store

Atomic store with Release semantic allows re-ordering of unordered load/store before the store.
Implement it.

Revie

[MemoryDependency] Relax the re-ordering of atomic store and unordered load/store

Atomic store with Release semantic allows re-ordering of unordered load/store before the store.
Implement it.

Reviewers: reames
Reviewed By: reames
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D119844

show more ...

[MemoryDependency] Relax the re-ordering with volatile store.

Volatile store does not provide any special rules for reordering with
atomics. Usual must alias anaylsis is enough here.

This makes the

[MemoryDependency] Relax the re-ordering with volatile store.

Volatile store does not provide any special rules for reordering with
atomics. Usual must alias anaylsis is enough here.

This makes the bahavior similar to how volatile load is handled.

Reviewers: reames, nikic
Reviewed By: reames
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D119818

show more ...

[MemoryBuiltins] Remove isNoAliasFn() in favor of isNoAliasCall()

We currently have two similar implementations of this concept:
isNoAliasCall() only checks for the noalias return attribute.
isNoAli

[MemoryBuiltins] Remove isNoAliasFn() in favor of isNoAliasCall()

We currently have two similar implementations of this concept:
isNoAliasCall() only checks for the noalias return attribute.
isNoAliasFn() also checks for allocation functions.

We should switch to only checking the attribute. SLC is responsible
for inferring the noalias return attribute for non-new allocation
functions (with a missing case fixed in
https://github.com/llvm/llvm-project/commit/348bc76e3548c52dbcd442590ca0a7f5b09b7534).
For new, clang is responsible for setting the attribute,
if -fno-assume-sane-operator-new is not passed.

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

show more ...

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

[NFC] MemoryDependenceAnalysis cleanup.

1. Removed redundant includes,
2. Removed never defined and used `releaseMemory()`.
3. Fixed member functions names first letter case.
4. Renamed duplicate (i

[NFC] MemoryDependenceAnalysis cleanup.

1. Removed redundant includes,
2. Removed never defined and used `releaseMemory()`.
3. Fixed member functions names first letter case.
4. Renamed duplicate (in nested struct `NonLocalPointerInfo`) name
`NonLocalDeps` to `NonLocalDepsMap`.

Reviewed By: nikic

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

show more ...

[MemDep] Use BatchAA in more places (NFCI)

Previously, we already used BatchAA for individual simple pointer
dependency queries. This extends BatchAA usage for the non-local
case, so that only one B

[MemDep] Use BatchAA in more places (NFCI)

Previously, we already used BatchAA for individual simple pointer
dependency queries. This extends BatchAA usage for the non-local
case, so that only one BatchAA instance is used for all blocks,
instead of one instance per block.

Use of BatchAA is safe as IR cannot be modified during a MemDep
query.

show more ...

[AA] Support callCapturesBefore() on BatchAA (NFCI)

This is not expected to have any practical compile-time effect,
as the alias() calls inside callCapturesBefore() are rare. This
should still be su

[AA] Support callCapturesBefore() on BatchAA (NFCI)

This is not expected to have any practical compile-time effect,
as the alias() calls inside callCapturesBefore() are rare. This
should still be supported for API completeness, and might be
useful for reachability caching.

show more ...

[GVN] Clobber partially aliased loads.

Use offsets stored in `AliasResult` implemented in D98718.

Updated with fix of issue reported in https://reviews.llvm.org/D95543#2745161

Reviewed By: nikic

[GVN] Clobber partially aliased loads.

Use offsets stored in `AliasResult` implemented in D98718.

Updated with fix of issue reported in https://reviews.llvm.org/D95543#2745161

Reviewed By: nikic

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

show more ...

Revert "[GVN] Clobber partially aliased loads."

This reverts commit 6c570442318e2d3b8b13e95c2f2f588d71491acb.

It causes assertion errors due to widening atomic loads, and potentially causes miscomp

Revert "[GVN] Clobber partially aliased loads."

This reverts commit 6c570442318e2d3b8b13e95c2f2f588d71491acb.

It causes assertion errors due to widening atomic loads, and potentially causes miscompile elsewhere too. Repro, also posted to D95543:

```
$ cat repro.ll
; ModuleID = 'repro.ll'
source_filename = "repro.ll"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%struct.widget = type { i32 }
%struct.baz = type { i32, %struct.snork }
%struct.snork = type { %struct.spam }
%struct.spam = type { i32, i32 }

@global = external local_unnamed_addr global %struct.widget, align 4
@global.1 = external local_unnamed_addr global i8, align 1
@global.2 = external local_unnamed_addr global i32, align 4

define void @zot(%struct.baz* %arg) local_unnamed_addr align 2 {
bb:
%tmp = getelementptr inbounds %struct.baz, %struct.baz* %arg, i64 0, i32 1
%tmp1 = bitcast %struct.snork* %tmp to i64*
%tmp2 = load i64, i64* %tmp1, align 4
%tmp3 = getelementptr inbounds %struct.baz, %struct.baz* %arg, i64 0, i32 1, i32 0, i32 1
%tmp4 = icmp ugt i64 %tmp2, 4294967295
br label %bb5

bb5: ; preds = %bb14, %bb
%tmp6 = load i32, i32* %tmp3, align 4
%tmp7 = icmp ne i32 %tmp6, 0
%tmp8 = select i1 %tmp7, i1 %tmp4, i1 false
%tmp9 = zext i1 %tmp8 to i8
store i8 %tmp9, i8* @global.1, align 1
%tmp10 = load i32, i32* @global.2, align 4
switch i32 %tmp10, label %bb11 [
i32 1, label %bb12
i32 2, label %bb12
]

bb11: ; preds = %bb5
br label %bb14

bb12: ; preds = %bb5, %bb5
%tmp13 = load atomic i32, i32* getelementptr inbounds (%struct.widget, %struct.widget* @global, i64 0, i32 0) acquire, align 4
br label %bb14

bb14: ; preds = %bb12, %bb11
br label %bb5
}
$ opt -O2 repro.ll -disable-output
opt: /home/rupprecht/src/llvm-project/llvm/lib/Transforms/Utils/VNCoercion.cpp:496: llvm::Value *llvm::VNCoercion::getLoadValueForLoad(llvm::LoadInst *, unsigned int, llvm::Type *, llvm::Instruction *, const llvm::DataLayout &): Assertion `SrcVal->isSimple() && "Cannot widen volatile/atomic load!"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0. Program arguments: /home/rupprecht/dev/opt -O2 repro.ll -disable-output
...
```

show more ...

[GVN] Clobber partially aliased loads.

Use offsets stored in `AliasResult` implemented in D98718.

Reviewed By: nikic

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

[NFC][AA] Prepare to convert AliasResult to class with PartialAlias offset.

Main reason is preparation to transform AliasResult to class that contains
offset for PartialAlias case.

Reviewed By: asb

[NFC][AA] Prepare to convert AliasResult to class with PartialAlias offset.

Main reason is preparation to transform AliasResult to class that contains
offset for PartialAlias case.

Reviewed By: asbirlea

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

show more ...

Extract isVolatile helper on Instruction [NFCI]

We have this logic duplicated in several cases, none of which were exhaustive. Consolidate it in one place.

I don't believe this actually impacts be

Extract isVolatile helper on Instruction [NFCI]

We have this logic duplicated in several cases, none of which were exhaustive. Consolidate it in one place.

I don't believe this actually impacts behavior of the callers. I think they all filter their inputs such that their partial implementations were correct. If not, this might be fixing a cornercase bug.

show more ...

[MemDepAnalysis] Remove redundant comment.

Exact same comment is found 2 lines above.

[MemoryDependence] Fix invariant group store

Fix bug in MemoryDependence [and thus GVN] for invariant group.

Previously MemDep didn't verify that the store was storing into a
pointer rather than a

[MemoryDependence] Fix invariant group store

Fix bug in MemoryDependence [and thus GVN] for invariant group.

Previously MemDep didn't verify that the store was storing into a
pointer rather than a store simply using a pointer.

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

show more ...

[llvm] Use pop_back_val (NFC)

[Analysis] Use llvm::append_range (NFC)

[llvm] Use llvm::lower_bound and llvm::upper_bound (NFC)

[AA] Split up LocationSize::unknown()

Currently, we have some confusion in the codebase regarding the
meaning of LocationSize::unknown(): Some parts (including most of
BasicAA) assume that LocationS

[AA] Split up LocationSize::unknown()

Currently, we have some confusion in the codebase regarding the
meaning of LocationSize::unknown(): Some parts (including most of
BasicAA) assume that LocationSize::unknown() only allows accesses
after the base pointer. Some parts (various callers of AA) assume
that LocationSize::unknown() allows accesses both before and after
the base pointer (but within the underlying object).

This patch splits up LocationSize::unknown() into
LocationSize::afterPointer() and LocationSize::beforeOrAfterPointer()
to make this completely unambiguous. I tried my best to determine
which one is appropriate for all the existing uses.

The test changes in cs-cs.ll in particular illustrate a previously
clearly incorrect AA result: We were effectively assuming that
argmemonly functions were only allowed to access their arguments
after the passed pointer, but not before it. I'm pretty sure that
this was not intentional, and it's certainly not specified by
LangRef that way.

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

show more ...

[MemLoc] Require LocationSize argument (NFC)

When constructing a MemoryLocation by hand, require that a
LocationSize is explicitly specified. D91649 will split up
LocationSize::unknown() into two di

[MemLoc] Require LocationSize argument (NFC)

When constructing a MemoryLocation by hand, require that a
LocationSize is explicitly specified. D91649 will split up
LocationSize::unknown() into two different states, and callers
should make an explicit choice regarding the kind of MemoryLocation
they want to have.

show more ...