Skip to content

[BasicAA] Track nuw through decomposed expressions #106512

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 2, 2024
Merged

[BasicAA] Track nuw through decomposed expressions #106512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 21 additions & 10 deletions llvm/lib/Analysis/BasicAliasAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,24 +375,28 @@ struct LinearExpression {
APInt Scale;
APInt Offset;

/// True if all operations in this expression are NUW.
bool IsNUW;
/// True if all operations in this expression are NSW.
bool IsNSW;

LinearExpression(const CastedValue &Val, const APInt &Scale,
const APInt &Offset, bool IsNSW)
: Val(Val), Scale(Scale), Offset(Offset), IsNSW(IsNSW) {}
const APInt &Offset, bool IsNUW, bool IsNSW)
: Val(Val), Scale(Scale), Offset(Offset), IsNUW(IsNUW), IsNSW(IsNSW) {}

LinearExpression(const CastedValue &Val) : Val(Val), IsNSW(true) {
LinearExpression(const CastedValue &Val)
: Val(Val), IsNUW(true), IsNSW(true) {
unsigned BitWidth = Val.getBitWidth();
Scale = APInt(BitWidth, 1);
Offset = APInt(BitWidth, 0);
}

LinearExpression mul(const APInt &Other, bool MulIsNSW) const {
LinearExpression mul(const APInt &Other, bool MulIsNUW, bool MulIsNSW) const {
// The check for zero offset is necessary, because generally
// (X +nsw Y) *nsw Z does not imply (X *nsw Z) +nsw (Y *nsw Z).
bool NSW = IsNSW && (Other.isOne() || (MulIsNSW && Offset.isZero()));
return LinearExpression(Val, Scale * Other, Offset * Other, NSW);
bool NUW = IsNUW && (Other.isOne() || MulIsNUW);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Proof that mul nuw always distributes (unlike mul nsw): https://alive2.llvm.org/ce/z/MZd_JJ

return LinearExpression(Val, Scale * Other, Offset * Other, NUW, NSW);
}
};
}
Expand All @@ -408,7 +412,7 @@ static LinearExpression GetLinearExpression(

if (const ConstantInt *Const = dyn_cast<ConstantInt>(Val.V))
return LinearExpression(Val, APInt(Val.getBitWidth(), 0),
Val.evaluateWith(Const->getValue()), true);
Val.evaluateWith(Const->getValue()), true, true);

if (const BinaryOperator *BOp = dyn_cast<BinaryOperator>(Val.V)) {
if (ConstantInt *RHSC = dyn_cast<ConstantInt>(BOp->getOperand(1))) {
Expand Down Expand Up @@ -444,20 +448,22 @@ static LinearExpression GetLinearExpression(
E = GetLinearExpression(Val.withValue(BOp->getOperand(0), false), DL,
Depth + 1, AC, DT);
E.Offset += RHS;
E.IsNUW &= NUW;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Proof for add nuw associativity: https://alive2.llvm.org/ce/z/oiNosh

And counter-proof that the current NSW code here is incorrect: https://alive2.llvm.org/ce/z/Ga8-CT

We are missing a pre-condition: https://alive2.llvm.org/ce/z/wkP_yw

But this is unrelated to this patch...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have trouble actually coming up with a test case that currently gets miscompiled due to the NSW issue ... ideas welcome :)

E.IsNSW &= NSW;
break;
}
case Instruction::Sub: {
E = GetLinearExpression(Val.withValue(BOp->getOperand(0), false), DL,
Depth + 1, AC, DT);
E.Offset -= RHS;
E.IsNUW = false; // sub nuw x, y is not add nuw x, -y.
E.IsNSW &= NSW;
break;
}
case Instruction::Mul:
E = GetLinearExpression(Val.withValue(BOp->getOperand(0), false), DL,
Depth + 1, AC, DT)
.mul(RHS, NSW);
.mul(RHS, NUW, NSW);
break;
case Instruction::Shl:
// We're trying to linearize an expression of the kind:
Expand All @@ -472,6 +478,7 @@ static LinearExpression GetLinearExpression(
Depth + 1, AC, DT);
E.Offset <<= RHS.getLimitedValue();
E.Scale <<= RHS.getLimitedValue();
E.IsNUW &= NUW;
E.IsNSW &= NSW;
break;
}
Expand Down Expand Up @@ -697,7 +704,8 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL,
// If the integer type is smaller than the index size, it is implicitly
// sign extended or truncated to index size.
bool NUSW = GEPOp->hasNoUnsignedSignedWrap();
bool NonNeg = NUSW && GEPOp->hasNoUnsignedWrap();
bool NUW = GEPOp->hasNoUnsignedWrap();
bool NonNeg = NUSW && NUW;
unsigned Width = Index->getType()->getIntegerBitWidth();
unsigned SExtBits = IndexSize > Width ? IndexSize - Width : 0;
unsigned TruncBits = IndexSize < Width ? Width - IndexSize : 0;
Expand All @@ -706,9 +714,11 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL,

// Scale by the type size.
unsigned TypeSize = AllocTypeSize.getFixedValue();
LE = LE.mul(APInt(IndexSize, TypeSize), NUSW);
LE = LE.mul(APInt(IndexSize, TypeSize), NUW, NUSW);
Decomposed.Offset += LE.Offset.sext(MaxIndexSize);
APInt Scale = LE.Scale.sext(MaxIndexSize);
if (!LE.IsNUW)
Decomposed.NWFlags = Decomposed.NWFlags.withoutNoUnsignedWrap();

// If we already had an occurrence of this index variable, merge this
// scale into it. For example, we want to handle:
Expand All @@ -719,7 +729,8 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL,
areBothVScale(Decomposed.VarIndices[i].Val.V, LE.Val.V)) &&
Decomposed.VarIndices[i].Val.hasSameCastsAs(LE.Val)) {
Scale += Decomposed.VarIndices[i].Scale;
LE.IsNSW = false; // We cannot guarantee nsw for the merge.
// We cannot guarantee no-wrap for the merge.
LE.IsNSW = LE.IsNUW = false;
Decomposed.VarIndices.erase(Decomposed.VarIndices.begin() + i);
break;
}
Expand Down
103 changes: 103 additions & 0 deletions llvm/test/Analysis/BasicAA/gep-nuw-alias.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,106 @@ define void @both_var_idx(ptr %p, i64 %i, i64 %j) {

ret void
}

; CHECK-LABEL: add_no_nuw
; CHECK: MayAlias: i8* %gep, i8* %p
define i8 @add_no_nuw(ptr %p, i64 %n) {
store i8 3, ptr %p

%add = add i64 %n, 1
%gep = getelementptr nuw i8, ptr %p, i64 %add
%val = load i8, ptr %gep
ret i8 %val
}

; CHECK-LABEL: add_nuw
; CHECK: NoAlias: i8* %gep, i8* %p
define i8 @add_nuw(ptr %p, i64 %n) {
store i8 3, ptr %p

%add = add nuw i64 %n, 1
%gep = getelementptr nuw i8, ptr %p, i64 %add
%val = load i8, ptr %gep
ret i8 %val
}

; CHECK-LABEL: add_no_nuw
; CHECK: MayAlias: i8* %gep, i16* %p
define i8 @add_no_nuw_scale(ptr %p, i64 %n) {
store i16 3, ptr %p

%add = add i64 %n, 1
%gep = getelementptr nuw i16, ptr %p, i64 %add
%val = load i8, ptr %gep
ret i8 %val
}

; CHECK-LABEL: add_nuw
; CHECK: NoAlias: i8* %gep, i16* %p
define i8 @add_nuw_scale(ptr %p, i64 %n) {
store i16 3, ptr %p

%add = add nuw i64 %n, 1
%gep = getelementptr nuw i16, ptr %p, i64 %add
%val = load i8, ptr %gep
ret i8 %val
}

; CHECK-LABEL: sub_nuw
; CHECK: MayAlias: i8* %gep, i8* %p
define i8 @sub_nuw(ptr %p, i64 %n) {
store i8 3, ptr %p

%add = sub nuw i64 %n, 1
%gep = getelementptr nuw i8, ptr %p, i64 %add
%val = load i8, ptr %gep
ret i8 %val
}

; CHECK-LABEL: mul_no_nuw
; CHECK: MayAlias: i8* %gep, i16* %p
define i8 @mul_no_nuw(ptr %p, i64 %n) {
store i16 3, ptr %p

%add = add nuw i64 %n, 1
%mul = mul i64 %add, 2
%gep = getelementptr nuw i8, ptr %p, i64 %mul
%val = load i8, ptr %gep
ret i8 %val
}

; CHECK-LABEL: mul_nuw
; CHECK: NoAlias: i8* %gep, i16* %p
define i8 @mul_nuw(ptr %p, i64 %n) {
store i16 3, ptr %p

%add = add nuw i64 %n, 1
%mul = mul nuw i64 %add, 2
%gep = getelementptr nuw i8, ptr %p, i64 %mul
%val = load i8, ptr %gep
ret i8 %val
}

; CHECK-LABEL: shl_no_nuw
; CHECK: MayAlias: i8* %gep, i16* %p
define i8 @shl_no_nuw(ptr %p, i64 %n) {
store i16 3, ptr %p

%add = add nuw i64 %n, 1
%shl = shl i64 %add, 1
%gep = getelementptr nuw i8, ptr %p, i64 %shl
%val = load i8, ptr %gep
ret i8 %val
}

; CHECK-LABEL: shl_nuw
; CHECK: NoAlias: i8* %gep, i16* %p
define i8 @shl_nuw(ptr %p, i64 %n) {
store i16 3, ptr %p

%add = add nuw i64 %n, 1
%shl = shl nuw i64 %add, 1
%gep = getelementptr nuw i8, ptr %p, i64 %shl
%val = load i8, ptr %gep
ret i8 %val
}
Loading