Skip to content

Commit

Permalink
[Attributor] Replace AAValueSimplify with AAPotentialValues
Browse files Browse the repository at this point in the history
For the longest time we used `AAValueSimplify` and
`genericValueTraversal` to determine "potential values". This was
problematic for many reasons:
- We recomputed the result a lot as there was no caching for the 9
  locations calling `genericValueTraversal`.
- We added the idea of "intra" vs. "inter" procedural simplification
  only as an afterthought. `genericValueTraversal` did offer an option
  but `AAValueSimplify` did not. Thus, we might end up with "too much"
  simplification in certain situations and then gave up on it.
- Because `genericValueTraversal` was not a real `AA` we ended up with
  problems like the infinite recursion bug (#54981) as well as code
  duplication.

This patch introduces `AAPotentialValues` and replaces the
`AAValueSimplify` uses with it. `genericValueTraversal` is folded into
`AAPotentialValues` as are the instruction simplifications performed in
`AAValueSimplify` before. We further distinguish "intra" and "inter"
procedural simplification now.

`AAValueSimplify` was not deleted as we haven't ported the
re-materialization of instructions yet. There are other differences over
the former handling, e.g., we may not fold trivially foldable
instructions right now, e.g., `add i32 1, 1` is not folded to `i32 2`
but if an operand would be simplified to `i32 1` we would fold it still.

We are also even more aware of function/SCC boundaries in CGSCC passes,
which is good even if some tests look like they regress.

Fixes: llvm/llvm-project#54981

Note: A previous version was flawed and consequently reverted in
      6555558a80589d1c5a1154b92cc3af9495f8f86c.
  • Loading branch information
jdoerfert authored and memfrob committed Oct 5, 2022
1 parent bc822fc commit 6ff3811
Show file tree
Hide file tree
Showing 73 changed files with 4,557 additions and 2,856 deletions.
107 changes: 93 additions & 14 deletions llvm/include/llvm/Transforms/IPO/Attributor.h
Expand Up @@ -118,7 +118,9 @@
#include "llvm/IR/ConstantRange.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/InstIterator.h"
#include "llvm/IR/Instruction.h"
#include "llvm/IR/PassManager.h"
#include "llvm/IR/Value.h"
#include "llvm/Support/Alignment.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Casting.h"
Expand Down Expand Up @@ -155,6 +157,7 @@ namespace AA {
enum ValueScope : uint8_t {
Intraprocedural = 1,
Interprocedural = 2,
AnyScope = Intraprocedural | Interprocedural,
};

struct ValueAndContext : public std::pair<Value *, const Instruction *> {
Expand Down Expand Up @@ -217,12 +220,11 @@ Constant *getInitialValueForObj(Value &Obj, Type &Ty,
/// \returns True if \p Objects contains all assumed underlying objects, and
/// false if something went wrong and the objects could not be
/// determined.
bool getAssumedUnderlyingObjects(Attributor &A, const Value &Ptr,
SmallVectorImpl<Value *> &Objects,
const AbstractAttribute &QueryingAA,
const Instruction *CtxI,
bool &UsedAssumedInformation,
AA::ValueScope VS = Interprocedural);
bool getAssumedUnderlyingObjects(
Attributor &A, const Value &Ptr, SmallSetVector<Value *, 8> &Objects,
const AbstractAttribute &QueryingAA, const Instruction *CtxI,
bool &UsedAssumedInformation, AA::ValueScope VS = AA::Interprocedural,
SmallPtrSetImpl<Value *> *SeenObjects = nullptr);

/// Collect all potential values \p LI could read into \p PotentialValues. That
/// is, the only values read by \p LI are assumed to be known and all are in
Expand Down Expand Up @@ -305,6 +307,24 @@ struct DenseMapInfo<AA::ValueAndContext>
}
};

template <>
struct DenseMapInfo<AA::ValueScope> : public DenseMapInfo<unsigned char> {
using Base = DenseMapInfo<unsigned char>;
static inline AA::ValueScope getEmptyKey() {
return AA::ValueScope(Base::getEmptyKey());
}
static inline AA::ValueScope getTombstoneKey() {
return AA::ValueScope(Base::getTombstoneKey());
}
static unsigned getHashValue(const AA::ValueScope &S) {
return Base::getHashValue(S);
}

static bool isEqual(const AA::ValueScope &LHS, const AA::ValueScope &RHS) {
return Base::isEqual(LHS, RHS);
}
};

/// The value passed to the line option that defines the maximal initialization
/// chain length.
extern unsigned MaxInitializationChainLength;
Expand Down Expand Up @@ -1643,8 +1663,6 @@ struct Attributor {

/// Record that \p F is deleted after information was manifested.
void deleteAfterManifest(Function &F) {
errs() << "Delete " << F.getName() << " : " << (Configuration.DeleteFns)
<< "\n";
if (Configuration.DeleteFns)
ToBeDeletedFunctions.insert(&F);
}
Expand All @@ -1664,22 +1682,34 @@ struct Attributor {
/// return None, otherwise return `nullptr`.
Optional<Value *> getAssumedSimplified(const IRPosition &IRP,
const AbstractAttribute &AA,
bool &UsedAssumedInformation) {
return getAssumedSimplified(IRP, &AA, UsedAssumedInformation);
bool &UsedAssumedInformation,
AA::ValueScope S) {
return getAssumedSimplified(IRP, &AA, UsedAssumedInformation, S);
}
Optional<Value *> getAssumedSimplified(const Value &V,
const AbstractAttribute &AA,
bool &UsedAssumedInformation) {
bool &UsedAssumedInformation,
AA::ValueScope S) {
return getAssumedSimplified(IRPosition::value(V), AA,
UsedAssumedInformation);
UsedAssumedInformation, S);
}

/// If \p V is assumed simplified, return it, if it is unclear yet,
/// return None, otherwise return `nullptr`. Same as the public version
/// except that it can be used without recording dependences on any \p AA.
Optional<Value *> getAssumedSimplified(const IRPosition &V,
const AbstractAttribute *AA,
bool &UsedAssumedInformation);
bool &UsedAssumedInformation,
AA::ValueScope S);

/// Try to simplify \p IRP and in the scope \p S. If successful, true is
/// returned and all potential values \p IRP can take are put into \p Values.
/// If false is returned no other information is valid.
bool getAssumedSimplifiedValues(const IRPosition &IRP,
const AbstractAttribute *AA,
SmallVectorImpl<AA::ValueAndContext> &Values,
AA::ValueScope S,
bool &UsedAssumedInformation);

/// Register \p CB as a simplification callback.
/// `Attributor::getAssumedSimplified` will use these callbacks before
Expand Down Expand Up @@ -4409,6 +4439,10 @@ template <typename MemberTy> struct PotentialValuesState : AbstractState {
return *this;
}

bool contains(const MemberTy &V) const {
return !isValidState() ? true : Set.contains(V);
}

protected:
SetTy &getAssumedSet() {
assert(isValidState() && "This set shoud not be used when it is invalid!");
Expand Down Expand Up @@ -4490,9 +4524,12 @@ template <typename MemberTy> struct PotentialValuesState : AbstractState {
};

using PotentialConstantIntValuesState = PotentialValuesState<APInt>;
using PotentialLLVMValuesState =
PotentialValuesState<std::pair<AA::ValueAndContext, AA::ValueScope>>;

raw_ostream &operator<<(raw_ostream &OS,
const PotentialConstantIntValuesState &R);
raw_ostream &operator<<(raw_ostream &OS, const PotentialLLVMValuesState &R);

/// An abstract interface for potential values analysis.
///
Expand All @@ -4508,7 +4545,7 @@ raw_ostream &operator<<(raw_ostream &OS,
/// 2. We tried to initialize on a Value that we cannot handle (e.g. an
/// operator we do not currently handle).
///
/// TODO: Support values other than constant integers.
/// For non constant integers see AAPotentialValues.
struct AAPotentialConstantValues
: public StateWrapper<PotentialConstantIntValuesState, AbstractAttribute> {
using Base = StateWrapper<PotentialConstantIntValuesState, AbstractAttribute>;
Expand Down Expand Up @@ -4562,6 +4599,48 @@ struct AAPotentialConstantValues
static const char ID;
};

struct AAPotentialValues
: public StateWrapper<PotentialLLVMValuesState, AbstractAttribute> {
using Base = StateWrapper<PotentialLLVMValuesState, AbstractAttribute>;
AAPotentialValues(const IRPosition &IRP, Attributor &A) : Base(IRP) {}

/// See AbstractAttribute::getState(...).
PotentialLLVMValuesState &getState() override { return *this; }
const PotentialLLVMValuesState &getState() const override { return *this; }

/// Create an abstract attribute view for the position \p IRP.
static AAPotentialValues &createForPosition(const IRPosition &IRP,
Attributor &A);

/// Extract the single value in \p Values if any.
static Value *getSingleValue(Attributor &A, const AbstractAttribute &AA,
const IRPosition &IRP,
SmallVectorImpl<AA::ValueAndContext> &Values);

/// See AbstractAttribute::getName()
const std::string getName() const override { return "AAPotentialValues"; }

/// See AbstractAttribute::getIdAddr()
const char *getIdAddr() const override { return &ID; }

/// This function should return true if the type of the \p AA is
/// AAPotentialValues
static bool classof(const AbstractAttribute *AA) {
return (AA->getIdAddr() == &ID);
}

/// Unique ID (due to the unique address)
static const char ID;

private:
virtual bool
getAssumedSimplifiedValues(Attributor &A,
SmallVectorImpl<AA::ValueAndContext> &Values,
AA::ValueScope) const = 0;

friend struct Attributor;
};

/// An abstract interface for all noundef attributes.
struct AANoUndef
: public IRAttribute<Attribute::NoUndef,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
Expand Up @@ -759,7 +759,8 @@ class AMDGPUAttributor : public ModulePass {
AMDGPUInformationCache InfoCache(M, AG, Allocator, nullptr, *TM);
DenseSet<const char *> Allowed(
{&AAAMDAttributes::ID, &AAUniformWorkGroupSize::ID,
&AAAMDFlatWorkGroupSize::ID, &AACallEdges::ID, &AAPointerInfo::ID});
&AAPotentialValues::ID, &AAAMDFlatWorkGroupSize::ID, &AACallEdges::ID,
&AAPointerInfo::ID});

AttributorConfig AC(CGUpdater);
AC.Allowed = &Allowed;
Expand Down

0 comments on commit 6ff3811

Please sign in to comment.