Skip to content

[mlir][IntRange] Poison support in int-range analysis #152932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion mlir/include/mlir/Dialect/Arith/Transforms/Passes.td
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def ArithIntRangeOpts : Pass<"int-range-optimizations"> {
// Explicitly depend on "arith" because this pass could create operations in
// `arith` out of thin air in some cases.
let dependentDialects = [
"::mlir::arith::ArithDialect"
"::mlir::arith::ArithDialect",
"::mlir::ub::UBDialect"
];
}

Expand Down
1 change: 1 addition & 0 deletions mlir/include/mlir/Dialect/UB/IR/UBOps.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "mlir/Bytecode/BytecodeOpInterface.h"
#include "mlir/IR/Dialect.h"
#include "mlir/IR/OpImplementation.h"
#include "mlir/Interfaces/InferIntRangeInterface.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"

#include "mlir/Dialect/UB/IR/UBOpsInterfaces.h.inc"
Expand Down
6 changes: 4 additions & 2 deletions mlir/include/mlir/Dialect/UB/IR/UBOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
#ifndef MLIR_DIALECT_UB_IR_UBOPS_TD
#define MLIR_DIALECT_UB_IR_UBOPS_TD

include "mlir/Interfaces/SideEffectInterfaces.td"
include "mlir/IR/AttrTypeBase.td"
include "mlir/Interfaces/InferIntRangeInterface.td"
include "mlir/Interfaces/SideEffectInterfaces.td"

include "UBOpsInterfaces.td"

Expand Down Expand Up @@ -39,7 +40,8 @@ def PoisonAttr : UB_Attr<"Poison", "poison", [PoisonAttrInterface]> {
// PoisonOp
//===----------------------------------------------------------------------===//

def PoisonOp : UB_Op<"poison", [ConstantLike, Pure]> {
def PoisonOp : UB_Op<"poison", [ConstantLike, Pure,
DeclareOpInterfaceMethods<InferIntRangeInterface, ["inferResultRanges"]>]> {
let summary = "Poisoned constant operation.";
let description = [{
The `poison` operation materializes a compile-time poisoned constant value
Expand Down
15 changes: 15 additions & 0 deletions mlir/include/mlir/Interfaces/InferIntRangeInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ class ConstantIntRanges {
/// The maximum value of an integer when it is interpreted as signed.
const APInt &smax() const;

/// Get the bitwidth of the ranges.
unsigned getBitWidth() const;

/// Return the bitwidth that should be used for integer ranges describing
/// `type`. For concrete integer types, this is their bitwidth, for `index`,
/// this is the internal storage bitwidth of `index` attributes, and for
Expand All @@ -62,6 +65,10 @@ class ConstantIntRanges {
/// sint_max(width)].
static ConstantIntRanges maxRange(unsigned bitwidth);

/// Create a poisoned range, i.e. a range that represents no valid integer
/// values.
static ConstantIntRanges poison(unsigned bitwidth);

/// Create a `ConstantIntRanges` with a constant value - that is, with the
/// bounds [value, value] for both its signed interpretations.
static ConstantIntRanges constant(const APInt &value);
Expand Down Expand Up @@ -96,6 +103,14 @@ class ConstantIntRanges {
/// value.
std::optional<APInt> getConstantValue() const;

/// Returns true if signed range is poisoned, i.e. no valid signed value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I agree with "no valid value" as the semantics here. We need to be much clearer that a poisoned range is one that's the result of undefined behavior, and this is assumed to be impossible.

/// can be represented.
bool isSignedPoison() const;

/// Returns true if unsigned range is poisoned, i.e. no valid unsigned value
/// can be represented.
bool isUnsignedPoison() const;

friend raw_ostream &operator<<(raw_ostream &os,
const ConstantIntRanges &range);

Expand Down
25 changes: 24 additions & 1 deletion mlir/lib/Dialect/Arith/Transforms/IntRangeOptimizations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "mlir/Analysis/DataFlow/DeadCodeAnalysis.h"
#include "mlir/Analysis/DataFlow/IntegerRangeAnalysis.h"
#include "mlir/Dialect/Arith/IR/Arith.h"
#include "mlir/Dialect/UB/IR/UBOps.h"
#include "mlir/Dialect/Utils/StaticValueUtils.h"
#include "mlir/IR/IRMapping.h"
#include "mlir/IR/Matchers.h"
Expand Down Expand Up @@ -46,6 +47,16 @@ static std::optional<APInt> getMaybeConstantValue(DataFlowSolver &solver,
return inferredRange.getConstantValue();
}

static bool isPoison(DataFlowSolver &solver, Value value) {
auto *maybeInferredRange =
solver.lookupState<IntegerValueRangeLattice>(value);
if (!maybeInferredRange || maybeInferredRange->getValue().isUninitialized())
return false;
const ConstantIntRanges &inferredRange =
maybeInferredRange->getValue().getValue();
return inferredRange.isSignedPoison() && inferredRange.isUnsignedPoison();
}

static void copyIntegerRange(DataFlowSolver &solver, Value oldVal,
Value newVal) {
assert(oldVal.getType() == newVal.getType() &&
Expand All @@ -63,6 +74,17 @@ LogicalResult maybeReplaceWithConstant(DataFlowSolver &solver,
RewriterBase &rewriter, Value value) {
if (value.use_empty())
return failure();

if (isPoison(solver, value)) {
Value poison =
ub::PoisonOp::create(rewriter, value.getLoc(), value.getType());
if (solver.lookupState<dataflow::IntegerValueRangeLattice>(poison))
solver.eraseState(poison);
copyIntegerRange(solver, value, poison);
rewriter.replaceAllUsesWith(value, poison);
return success();
}

std::optional<APInt> maybeConstValue = getMaybeConstantValue(solver, value);
if (!maybeConstValue.has_value())
return failure();
Expand Down Expand Up @@ -131,7 +153,8 @@ struct MaterializeKnownConstantValues : public RewritePattern {
return failure();

auto needsReplacing = [&](Value v) {
return getMaybeConstantValue(solver, v).has_value() && !v.use_empty();
return (getMaybeConstantValue(solver, v) || isPoison(solver, v)) &&
!v.use_empty();
};
bool hasConstantResults = llvm::any_of(op->getResults(), needsReplacing);
if (op->getNumRegions() == 0)
Expand Down
6 changes: 6 additions & 0 deletions mlir/lib/Dialect/UB/IR/UBOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ Operation *UBDialect::materializeConstant(OpBuilder &builder, Attribute value,

OpFoldResult PoisonOp::fold(FoldAdaptor /*adaptor*/) { return getValue(); }

void PoisonOp::inferResultRanges(ArrayRef<ConstantIntRanges> /*argRanges*/,
SetIntRangeFn setResultRange) {
unsigned width = ConstantIntRanges::getStorageBitwidth(getType());
setResultRange(getResult(), ConstantIntRanges::poison(width));
}

#include "mlir/Dialect/UB/IR/UBOpsInterfaces.cpp.inc"

#define GET_ATTRDEF_CLASSES
Expand Down
93 changes: 81 additions & 12 deletions mlir/lib/Interfaces/InferIntRangeInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const APInt &ConstantIntRanges::smin() const { return sminVal; }

const APInt &ConstantIntRanges::smax() const { return smaxVal; }

unsigned ConstantIntRanges::getBitWidth() const { return umin().getBitWidth(); }

unsigned ConstantIntRanges::getStorageBitwidth(Type type) {
type = getElementTypeOrSelf(type);
if (type.isIndex())
Expand All @@ -42,6 +44,21 @@ ConstantIntRanges ConstantIntRanges::maxRange(unsigned bitwidth) {
return fromUnsigned(APInt::getZero(bitwidth), APInt::getMaxValue(bitwidth));
}

ConstantIntRanges ConstantIntRanges::poison(unsigned bitwidth) {
if (bitwidth == 0) {
auto zero = APInt::getZero(0);
return {zero, zero, zero, zero};
}

// Poison is represented by an empty range.
auto zero = APInt::getZero(bitwidth);
auto one = zero + 1;
auto onem = zero - 1;
// For i1 the valid unsigned range is [0, 1] and the valid signed range
// is [-1, 0].
return {one, zero, zero, onem};
}

ConstantIntRanges ConstantIntRanges::constant(const APInt &value) {
return {value, value, value, value};
}
Expand Down Expand Up @@ -85,15 +102,37 @@ ConstantIntRanges
ConstantIntRanges::rangeUnion(const ConstantIntRanges &other) const {
// "Not an integer" poisons everything and also cannot be fed to comparison
// operators.
if (umin().getBitWidth() == 0)
if (getBitWidth() == 0)
return *this;
if (other.umin().getBitWidth() == 0)
if (other.getBitWidth() == 0)
return other;

const APInt &uminUnion = umin().ult(other.umin()) ? umin() : other.umin();
const APInt &umaxUnion = umax().ugt(other.umax()) ? umax() : other.umax();
const APInt &sminUnion = smin().slt(other.smin()) ? smin() : other.smin();
const APInt &smaxUnion = smax().sgt(other.smax()) ? smax() : other.smax();
APInt uminUnion;
APInt umaxUnion;
APInt sminUnion;
APInt smaxUnion;

if (isUnsignedPoison()) {
uminUnion = other.umin();
umaxUnion = other.umax();
} else if (other.isUnsignedPoison()) {
uminUnion = umin();
umaxUnion = umax();
} else {
uminUnion = umin().ult(other.umin()) ? umin() : other.umin();
umaxUnion = umax().ugt(other.umax()) ? umax() : other.umax();
}

if (isSignedPoison()) {
sminUnion = other.smin();
smaxUnion = other.smax();
} else if (other.isSignedPoison()) {
sminUnion = smin();
smaxUnion = smax();
} else {
sminUnion = smin().slt(other.smin()) ? smin() : other.smin();
smaxUnion = smax().sgt(other.smax()) ? smax() : other.smax();
}

return {uminUnion, umaxUnion, sminUnion, smaxUnion};
}
Expand All @@ -102,15 +141,37 @@ ConstantIntRanges
ConstantIntRanges::intersection(const ConstantIntRanges &other) const {
// "Not an integer" poisons everything and also cannot be fed to comparison
// operators.
if (umin().getBitWidth() == 0)
if (getBitWidth() == 0)
return *this;
if (other.umin().getBitWidth() == 0)
if (other.getBitWidth() == 0)
return other;

const APInt &uminIntersect = umin().ugt(other.umin()) ? umin() : other.umin();
const APInt &umaxIntersect = umax().ult(other.umax()) ? umax() : other.umax();
const APInt &sminIntersect = smin().sgt(other.smin()) ? smin() : other.smin();
const APInt &smaxIntersect = smax().slt(other.smax()) ? smax() : other.smax();
APInt uminIntersect;
APInt umaxIntersect;
APInt sminIntersect;
APInt smaxIntersect;

if (isUnsignedPoison()) {
uminIntersect = umin();
umaxIntersect = umax();
} else if (other.isUnsignedPoison()) {
uminIntersect = other.umin();
umaxIntersect = other.umax();
} else {
uminIntersect = umin().ugt(other.umin()) ? umin() : other.umin();
umaxIntersect = umax().ult(other.umax()) ? umax() : other.umax();
}

if (isSignedPoison()) {
sminIntersect = smin();
smaxIntersect = smax();
} else if (other.isSignedPoison()) {
sminIntersect = other.smin();
smaxIntersect = other.smax();
} else {
sminIntersect = smin().sgt(other.smin()) ? smin() : other.smin();
smaxIntersect = smax().slt(other.smax()) ? smax() : other.smax();
}

return {uminIntersect, umaxIntersect, sminIntersect, smaxIntersect};
}
Expand All @@ -124,6 +185,14 @@ std::optional<APInt> ConstantIntRanges::getConstantValue() const {
return std::nullopt;
}

bool ConstantIntRanges::isSignedPoison() const {
return getBitWidth() > 0 && smin().sgt(smax());
}

bool ConstantIntRanges::isUnsignedPoison() const {
return getBitWidth() > 0 && umin().ugt(umax());
}

raw_ostream &mlir::operator<<(raw_ostream &os, const ConstantIntRanges &range) {
os << "unsigned : [";
range.umin().print(os, /*isSigned*/ false);
Expand Down
Loading