Skip to content

Commit

Permalink
[instsimplify] Factor out a helper for alloca bounds checking [NFC]
Browse files Browse the repository at this point in the history
At the moment, this just groups comments with a reasonably named predicate, but I plan to add other cases to this in the near future.
  • Loading branch information
preames committed Feb 18, 2022
1 parent 6877ec4 commit f6510e6
Showing 1 changed file with 35 additions and 30 deletions.
65 changes: 35 additions & 30 deletions llvm/lib/Analysis/InstructionSimplify.cpp
Expand Up @@ -2507,6 +2507,36 @@ static Value *ExtractEquivalentCondition(Value *V, CmpInst::Predicate Pred,
return nullptr;
}

/// Return true if V1 and V2 are each the base of some distict storage region
/// [V, object_size(V)] which do not overlap. Note that zero sized regions
/// *are* possible, and that zero sized regions do not overlap with any other.
static bool HaveNonOverlappingStorage(const Value *V1, const Value *V2) {
// Global variables always exist, so they always exist during the lifetime
// of each other and all allocas. Two different allocas usually have
// different addresses...
//
// However, if there's an @llvm.stackrestore dynamically in between two
// allocas, they may have the same address. It's tempting to reduce the
// scope of the problem by only looking at *static* allocas here. That would
// cover the majority of allocas while significantly reducing the likelihood
// of having an @llvm.stackrestore pop up in the middle. However, it's not
// actually impossible for an @llvm.stackrestore to pop up in the middle of
// an entry block. Also, if we have a block that's not attached to a
// function, we can't tell if it's "static" under the current definition.
// Theoretically, this problem could be fixed by creating a new kind of
// instruction kind specifically for static allocas. Such a new instruction
// could be required to be at the top of the entry block, thus preventing it
// from being subject to a @llvm.stackrestore. Instcombine could even
// convert regular allocas into these special allocas. It'd be nifty.
// However, until then, this problem remains open.
//
// So, we'll assume that two non-empty allocas have different addresses
// for now.
//
return isa<AllocaInst>(V1) &&
(isa<AllocaInst>(V2) || isa<GlobalVariable>(V2));
}

// A significant optimization not implemented here is assuming that alloca
// addresses are not equal to incoming argument values. They don't *alias*,
// as we say, but that doesn't mean they aren't equal, so we take a
Expand Down Expand Up @@ -2599,36 +2629,11 @@ computePointerICmp(CmpInst::Predicate Pred, Value *LHS, Value *RHS,
// Various optimizations for (in)equality comparisons.
if (Pred == CmpInst::ICMP_EQ || Pred == CmpInst::ICMP_NE) {
// Different non-empty allocations that exist at the same time have
// different addresses (if the program can tell). Global variables always
// exist, so they always exist during the lifetime of each other and all
// allocas. Two different allocas usually have different addresses...
//
// However, if there's an @llvm.stackrestore dynamically in between two
// allocas, they may have the same address. It's tempting to reduce the
// scope of the problem by only looking at *static* allocas here. That would
// cover the majority of allocas while significantly reducing the likelihood
// of having an @llvm.stackrestore pop up in the middle. However, it's not
// actually impossible for an @llvm.stackrestore to pop up in the middle of
// an entry block. Also, if we have a block that's not attached to a
// function, we can't tell if it's "static" under the current definition.
// Theoretically, this problem could be fixed by creating a new kind of
// instruction kind specifically for static allocas. Such a new instruction
// could be required to be at the top of the entry block, thus preventing it
// from being subject to a @llvm.stackrestore. Instcombine could even
// convert regular allocas into these special allocas. It'd be nifty.
// However, until then, this problem remains open.
//
// So, we'll assume that two non-empty allocas have different addresses
// for now.
//
// With all that, if the offsets are within the bounds of their allocations
// (and not one-past-the-end! so we can't use inbounds!), and their
// allocations aren't the same, the pointers are not equal.
//
// Note that it's not necessary to check for LHS being a global variable
// address, due to canonicalization and constant folding.
if (isa<AllocaInst>(LHS) &&
(isa<AllocaInst>(RHS) || isa<GlobalVariable>(RHS))) {
// different addresses (if the program can tell). If the offsets are
// within the bounds of their allocations (and not one-past-the-end!
// so we can't use inbounds!), and their allocations aren't the same,
// the pointers are not equal.
if (HaveNonOverlappingStorage(LHS, RHS)) {
uint64_t LHSSize, RHSSize;
ObjectSizeOpts Opts;
Opts.EvalMode = ObjectSizeOpts::Mode::Min;
Expand Down

0 comments on commit f6510e6

Please sign in to comment.