| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
|
|
It seems TypeSize is currently broken in the sense that:
TypeSize::Fixed(4) + TypeSize::Scalable(4) => TypeSize::Fixed(8)
without failing its assert that explicitly tests for this case:
assert(LHS.Scalable == RHS.Scalable && ...);
The reason this fails is that `Scalable` is a static method of class
TypeSize,
and LHS and RHS are both objects of class TypeSize. So this is
evaluating
if the pointer to the function Scalable == the pointer to the function
Scalable,
which is always true because LHS and RHS have the same class.
This patch fixes the issue by renaming `TypeSize::Scalable` ->
`TypeSize::getScalable`, as well as `TypeSize::Fixed` to
`TypeSize::getFixed`,
so that it no longer clashes with the variable in
FixedOrScalableQuantity.
The new methods now also better match the coding standard, which
specifies that:
* Variable names should be nouns (as they represent state)
* Function names should be verb phrases (as they represent actions)
|
|
In most cases we do not actually have to perform an index size
adjustment. Don't perform any APInt operations in that case.
|
|
The current code incorrectly assumed that the absolute variable index
needs to be at least 1, if the variable is != 0. This is incorrect, in
case multiplying with Scale wraps.
The code below already checks for wrapping properly, so just remove the
incorrect assignment.
Fixes https://github.com/llvm/llvm-project/issues/72831.
|
|
I thought DT is required in BasicAA, but apparently it can be null
in unit tests at least. This should fix the ubsan bot failures.
|
|
For calls, we are only interested in captures before the call, not
captures by the call itself -- arguments that get passed to the call are
checked explicitly.
In particular, the current implementation is not optimal if the pointer
is captured via a readonly argument -- in that case, we know that even
if the argument is captured, the call will not modify the argument (at
least not via that argument).
Make this more precise by renaming to isCapturedBefore() and adding an
OrAt argument that allows us to toggle whether to consider captures in
the instruction itself or not.
|
|
(#71066)
Unfortunately the commit (D123162) introduced a mis-compile
(https://github.com/llvm/llvm-project/issues/70547), which wasn't fixed
by the alternative fix (c0de28b92e98acbeb73)
I think as long as the call considered as ephemeral is not removed, we
need to be conservative. To address the correctness issue quickly, I
think we should revert the patch (as this patch does, it doens't revert
cleanly)
This reverts commit 17fdaccccfad9b143e4aadbcdda7f645de127153.
Fixes https://github.com/llvm/llvm-project/issues/70547
|
|
TypeSize::{Fixed,Scalable}. NFC
|
|
This is the first of a series of patch to improve Alias Analysis on
Scalable quantities.
Keep Scalable information from TypeSize which
will be used in Alias Analysis.
|
|
|
|
Prefer this over UnknownSize, especially once the return value
switches to something other than uint64_t.
|
|
To allow using it in places where these may not be available.
|
|
When merging scales of `LinearExpression` that have common index
variables, we cannot guarantee the NSW flag still applies to the merged
expression.
Fixes #69096.
|
|
Historically, AA implementations chained to a following implementation
to answer recursive queries. This is no longer the case, but the legacy
lives on in a confusing phrasing of the return-a-conservative-value
paths. Let's just return "don't know" directly, where appropriate; the
current two-step way is confusing.
Differential Revision: https://reviews.llvm.org/D149100
|
|
We currently preserve the nsw flag when negating scales, which is
incorrect for INT_MIN.
However, just dropping the NSW flag in this case makes BasicAA
behavior unreliable and asymmetric, because we may or may not
drop the NSW flag depending on which side gets subtracted.
Instead, leave the Scale alone and add an additional IsNegated flag,
which indicates that the whole VarIndex should be interpreted as a
subtraction. This allows us to retain the NSW flag.
When accumulating the offset range, we need to use subtraction
instead of adding for IsNegated indices. Everything else works on
the absolute value of the scale, so the negation does not matter
there.
Fixes https://github.com/llvm/llvm-project/issues/63266.
Differential Revision: https://reviews.llvm.org/D153270
|
|
This is an alternative to D153464. BasicAA currently assumes that
an unescaped alloca cannot be read through non-nocapture arguments
of a call, based on the argument that if the argument were based on
the alloca, it would not be unescaped.
This currently fails in the case where the call is an ephemeral value
and as such does not count as a capture. It also happens for calls
that are readonly+nounwind+void, though that case tends to not matter
in practice, because such calls will get DCEd anyway.
Differential Revision: https://reviews.llvm.org/D153511
|
|
The last use was removed by:
commit fa6ea7a419f37befbed04368bcb8af4c718facbb
Author: Arthur Eubanks <aeubanks@google.com>
Date: Mon Mar 20 11:18:35 2023 -0700
Once we remove it, createLegacyPMAAResults and createLegacyPMAAResults
become unused, so this patch removes them as well.
Differential Revision: https://reviews.llvm.org/D151787
|
|
Before this change, we call getUnderlyingObject on each separate_storage
operand on every alias() call (potentially requiring lots of pointer
chasing). Instead, we rewrite the assumptions in instcombine to do this
pointer-chasing once.
We still leave the getUnderlyingObject calls in alias(), just expecting
them to be no-ops much of the time. This is relatively fast (just a
couple dyn_casts with no pointer chasing) and avoids making alias
analysis results depend on whether or not instcombine has been run.
Differential Revision: https://reviews.llvm.org/D144933
|
|
|
|
This change is one of a series to implement the discussion from
https://reviews.llvm.org/D141134.
|
|
This operand bundle on an assume informs alias analysis that the
arguments point to regions of memory that were allocated separately
(i.e. different heap allocations, different allocas, or different
globals).
As a safety measure, we leave the analysis flag-disabled by default.
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D136514
|
|
All current analyses ignore the context. We make the argument mandatory
for analyses, but optional for the query interface.
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D136512
|
|
BasicAA currently has an optional dependency on the PhiValues
analysis. However, at least with our current pipeline setup, we
never actually make use of it. It's possible that this used to work
with the legacy pass manager, but I'm not sure of that either.
Given that this analysis has not actually been in use for a long
time, and nobody noticed or complained, I think we should drop
support for it and focus on one code path. It is worth noting that
analysis quality for the non-PhiValues case has significantly
improved in the meantime.
If we really wanted to make use of PhiValues, the right way would
probably be to pass it in via AAQI in places we want to use it,
rather than using an optional pass manager dependency (which are
an unpredictable PITA and should really only ever be used for
analyses that are only preserved and not used).
Differential Revision: https://reviews.llvm.org/D139719
|
|
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
|
|
We can skip such incoming values. This was already done by
PhiValues if present, but we can also do this without the
additional analysis.
|
|
Succs can be empty here if a phi predecessor is unreachable.
Fixes https://github.com/llvm/llvm-project/issues/59360
|
|
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
|
|
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D139229
|
|
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
|
|
Move the MayBeCrossIteration flag from BasicAA into AAQI. This is
in preparation for exposing it to users of the AA API.
|
|
This switches everything to use the memory attribute proposed in
https://discourse.llvm.org/t/rfc-unify-memory-effect-attributes/65579.
The old argmemonly, inaccessiblememonly and inaccessiblemem_or_argmemonly
attributes are dropped. The readnone, readonly and writeonly attributes
are restricted to parameters only.
The old attributes are auto-upgraded both in bitcode and IR.
The bitcode upgrade is a policy requirement that has to be retained
indefinitely. The IR upgrade is mainly there so it's not necessary
to update all tests using memory attributes in this patch, which
is already large enough. We could drop that part after migrating
tests, or retain it longer term, to make it easier to import IR
from older LLVM versions.
High-level Function/CallBase APIs like doesNotAccessMemory() or
setDoesNotAccessMemory() are mapped transparently to the memory
attribute. Code that directly manipulates attributes (e.g. via
AttributeList) on the other hand needs to switch to working with
the memory attribute instead.
Differential Revision: https://reviews.llvm.org/D135780
|
|
pointsToConstantMemory().
The pointsToConstantMemory() method returns true only if the memory pointed to
by the memory location is globally invariant. However, the LLVM memory model
also has the semantic notion of *locally-invariant*: memory that is known to be
invariant for the life of the SSA value representing that pointer. The most
common example of this is a pointer argument that is marked readonly noalias,
which the Rust compiler frequently emits.
It'd be desirable for LLVM to treat locally-invariant memory the same way as
globally-invariant memory when it's safe to do so. This patch implements that,
by introducing the concept of a *ModRefInfo mask*. A ModRefInfo mask is a bound
on the Mod/Ref behavior of an instruction that writes to a memory location,
based on the knowledge that the memory is globally-constant memory (in which
case the mask is NoModRef) or locally-constant memory (in which case the mask
is Ref). ModRefInfo values for an instruction can be combined with the
ModRefInfo mask by simply using the & operator. Where appropriate, this patch
has modified uses of pointsToConstantMemory() to instead examine the mask.
The most notable optimization change I noticed with this patch is that now
redundant loads from readonly noalias pointers can be eliminated across calls,
even when the pointer is captured. Internally, before this patch,
AliasAnalysis was assigning Ref to reads from constant memory; now AA can
assign NoModRef, which is a tighter bound.
Differential Revision: https://reviews.llvm.org/D136659
|
|
Rather than switching to a new AAQI instance with empty cache when
MayBeCrossIteration is toggled, include the value in the cache key.
The implementation redundantly include the information in both sides
of the pair, but that seems simpler than trying to store it only on
one side.
Differential Revision: https://reviews.llvm.org/D136175
|
|
The writeonly attribute for memset_pattern16 (and other referenced
libcalls) is being added by InferFunctionAttrs nowadays. No need
to special-case it here.
|
|
|
|
When looking through phis, BasicAA has to guard against the
possibility that values from two separate cycle iterations are
being compared -- in this case, even though the SSA values may
be the same, they cannot be considered as equal.
This is currently done by keeping a set of VisitedPhiBBs for any
phis we looked through, and then checking whether the relevant
instruction is reachable from one of the phis.
This patch replaces this set with a single flag. If the flag is
set, then we will not assume equality for any instruction part
of a cycle. While this is nominally less accurate, it makes
essentially no difference in practice. Here are the AA stats
for test-suite:
aa.NumMayAlias | 3072005 | 3072016
aa.NumMustAlias | 337858 | 337854
aa.NumNoAlias | 13255345 | 13255349
The motivation for the change is to expose the MayBeCrossIteration
flag to AA users, which will allow fixing miscompiles related to
incorrect handling of cross-iteration AA queries.
Differential Revision: https://reviews.llvm.org/D136174
|
|
Follow up on D135962, renaming the method name to match the new
type name.
|
|
Followup to D135962 to rename remaining uses of
FunctionModRefBehavior to MemoryEffects. Does not touch API names
yet, but also updates variables names FMRB/MRB to ME, to match the
new type name.
|
|
When looking for underlying objects, if we encounter one that we
have already seen, then we should skip it (as it has already been
checked) rather than bail out. In particular, this adds support
for the case where we have a loop use of a phi recurrence.
|
|
If we have translated across a cycle backedge, the same SSA value
for the condition might be referring to two different loop iterations.
Use the isValueEqualInPotentialCycles() helper to avoid assuming
equality in that case.
|
|
Currently, AAResultBase (from which alias analysis providers inherit)
stores a reference back to the AAResults aggregation it is part of,
so it can perform recursive alias analysis queries via
getBestAAResults().
This patch removes the back-reference from AAResultBase to AAResults,
and instead passes the used aggregation through the AAQueryInfo.
This can be used to perform recursive AA queries using the full
aggregation.
Differential Revision: https://reviews.llvm.org/D94363
|
|
This is in preparation for D94363, as we will need AAQI to
perform the recursive call to the function variant.
|
|
|
|
The current implementation for call sites is pretty convoluted
when you take the underlying implementation of the used APIs
into account. We will query the call site attributes, and then
fall back to the function attributes while taking into account
operand bundles. However, getModRefBehavior() already has it's
own (more accurate) logic for combining call-site FMRB with
function FMRB.
Clean this up by extracting a function that only fetches FMRB
from attributes, which can be directly used in getModRefBehavior()
for functions, and needs to be combined with an operand-bundle
respecting fallback in the call site case.
One caveat (that makes this non-NFC) is that CallBase function
attribute lookups allow using attributes from functions with
mismatching signature. To ensure we don't regress quality, do
the same for the function FMRB fallback.
|
|
While we can't express this with attributes yet, we can model
these intrinsics as readonly + writing inaccessible memory (for
the control dependence) in FMRB. This way we don't need to
special-case them in getModRefInfo(), it falls out of the usual
logic.
|
|
Based on D130896, we can model operand bundles more precisely. In
addition to the baseline ModRefBehavior, a reading/clobbering operand
bundle may also read/write all locations. For example, a memcpy with
deopt bundle can read any memory, but only write argument memory.
This means that getModRefInfo() for memcpy with a pointer that does
not alias the arguments results in Ref, rather than ModRef, without
the need to implement any special handling.
Differential Revision: https://reviews.llvm.org/D130980
|
|
Currently, FunctionModRefBehavior tracks whether the function reads
or writes memory (ModRefInfo) and which locations it can access
(argmem, inaccessiblemem and other). This patch changes it to track
ModRef information per-location instead.
To give two examples of why this is useful:
* D117095 highlights a weakness of ModRef modelling in the presence
of operand bundles. For a memcpy call with deopt operand bundle,
we want to say that it can read any memory, but only write argument
memory. This would allow them to be treated like any other calls.
However, we currently can't express this and have to say that it
can read or write any memory.
* D127383 would ideally be modelled as a separate threadid location,
where threadid Refs outside pre-split coroutines can be ignored
(like other accesses to constant memory). The current representation
does not allow modelling this precisely.
The patch as implemented is intended to be NFC, but there are some
obvious opportunities for improvements and simplification. To fully
capitalize on this we would also want to change the way we represent
memory attributes on functions, but that's a larger change, and I
think it makes sense to separate out the FunctionModRefBehavior
refactoring.
Differential Revision: https://reviews.llvm.org/D130896
|
|
This call is expensive, so don't perform it for zero indices.
Also rename the variable to use Alloc rather than Alloca, this
doesn't have anything to do with allocas in particular.
|
|
According https://discourse.llvm.org/t/memoryssa-does-the-accessedbetween-support-scalable-vector-pointer/65052,
scalable vector support in BasicAA is currently essentially limited,
and should be improved effectively for a constant offset GEP if the scalable index is zero, eg:
getelementptr <vscale x 4 x i32>, ptr %p, i64 0, i64 %i
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D133567
|
|
With C++17 there is no Clang pedantic warning or MSVC C5051.
|
