release/19.x: [llvm] Fix __builtin_object_size interaction between Negative Offset … (#111827) #114786
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@hvdijk What do you think about merging this PR to the release branch? |
llvmbot
added
llvm:analysis
|
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-transforms Author: None (llvmbot) ChangesBackport 01a103b Requested by: @hvdijk Patch is 30.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114786.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Analysis/MemoryBuiltins.h b/llvm/include/llvm/Analysis/MemoryBuiltins.h
index bb282a1b73d320..a21f116db7e70d 100644
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -222,21 +222,43 @@ struct SizeOffsetAPInt : public SizeOffsetType<APInt, SizeOffsetAPInt> {
static bool known(const APInt &V) { return V.getBitWidth() > 1; }
};
+/// OffsetSpan - Used internally by \p ObjectSizeOffsetVisitor. Represents a
+/// point in memory as a pair of allocated bytes before and after it.
+struct OffsetSpan {
+ APInt Before; /// Number of allocated bytes before this point.
+ APInt After; /// Number of allocated bytes after this point.
+
+ OffsetSpan() = default;
+ OffsetSpan(APInt Before, APInt After) : Before(Before), After(After) {}
+
+ bool knownBefore() const { return known(Before); }
+ bool knownAfter() const { return known(After); }
+ bool anyKnown() const { return knownBefore() || knownAfter(); }
+ bool bothKnown() const { return knownBefore() && knownAfter(); }
+
+ bool operator==(const OffsetSpan &RHS) const {
+ return Before == RHS.Before && After == RHS.After;
+ }
+ bool operator!=(const OffsetSpan &RHS) const { return !(*this == RHS); }
+
+ static bool known(const APInt &V) { return V.getBitWidth() > 1; }
+};
+
/// Evaluate the size and offset of an object pointed to by a Value*
/// statically. Fails if size or offset are not known at compile time.
class ObjectSizeOffsetVisitor
- : public InstVisitor<ObjectSizeOffsetVisitor, SizeOffsetAPInt> {
+ : public InstVisitor<ObjectSizeOffsetVisitor, OffsetSpan> {
const DataLayout &DL;
const TargetLibraryInfo *TLI;
ObjectSizeOpts Options;
unsigned IntTyBits;
APInt Zero;
- SmallDenseMap<Instruction *, SizeOffsetAPInt, 8> SeenInsts;
+ SmallDenseMap<Instruction *, OffsetSpan, 8> SeenInsts;
unsigned InstructionsVisited;
APInt align(APInt Size, MaybeAlign Align);
- static SizeOffsetAPInt unknown() { return SizeOffsetAPInt(); }
+ static OffsetSpan unknown() { return OffsetSpan(); }
public:
ObjectSizeOffsetVisitor(const DataLayout &DL, const TargetLibraryInfo *TLI,
@@ -246,29 +268,30 @@ class ObjectSizeOffsetVisitor
// These are "private", except they can't actually be made private. Only
// compute() should be used by external users.
- SizeOffsetAPInt visitAllocaInst(AllocaInst &I);
- SizeOffsetAPInt visitArgument(Argument &A);
- SizeOffsetAPInt visitCallBase(CallBase &CB);
- SizeOffsetAPInt visitConstantPointerNull(ConstantPointerNull &);
- SizeOffsetAPInt visitExtractElementInst(ExtractElementInst &I);
- SizeOffsetAPInt visitExtractValueInst(ExtractValueInst &I);
- SizeOffsetAPInt visitGlobalAlias(GlobalAlias &GA);
- SizeOffsetAPInt visitGlobalVariable(GlobalVariable &GV);
- SizeOffsetAPInt visitIntToPtrInst(IntToPtrInst &);
- SizeOffsetAPInt visitLoadInst(LoadInst &I);
- SizeOffsetAPInt visitPHINode(PHINode &);
- SizeOffsetAPInt visitSelectInst(SelectInst &I);
- SizeOffsetAPInt visitUndefValue(UndefValue &);
- SizeOffsetAPInt visitInstruction(Instruction &I);
+ OffsetSpan visitAllocaInst(AllocaInst &I);
+ OffsetSpan visitArgument(Argument &A);
+ OffsetSpan visitCallBase(CallBase &CB);
+ OffsetSpan visitConstantPointerNull(ConstantPointerNull &);
+ OffsetSpan visitExtractElementInst(ExtractElementInst &I);
+ OffsetSpan visitExtractValueInst(ExtractValueInst &I);
+ OffsetSpan visitGlobalAlias(GlobalAlias &GA);
+ OffsetSpan visitGlobalVariable(GlobalVariable &GV);
+ OffsetSpan visitIntToPtrInst(IntToPtrInst &);
+ OffsetSpan visitLoadInst(LoadInst &I);
+ OffsetSpan visitPHINode(PHINode &);
+ OffsetSpan visitSelectInst(SelectInst &I);
+ OffsetSpan visitUndefValue(UndefValue &);
+ OffsetSpan visitInstruction(Instruction &I);
private:
- SizeOffsetAPInt findLoadSizeOffset(
- LoadInst &LoadFrom, BasicBlock &BB, BasicBlock::iterator From,
- SmallDenseMap<BasicBlock *, SizeOffsetAPInt, 8> &VisitedBlocks,
- unsigned &ScannedInstCount);
- SizeOffsetAPInt combineSizeOffset(SizeOffsetAPInt LHS, SizeOffsetAPInt RHS);
- SizeOffsetAPInt computeImpl(Value *V);
- SizeOffsetAPInt computeValue(Value *V);
+ OffsetSpan
+ findLoadOffsetRange(LoadInst &LoadFrom, BasicBlock &BB,
+ BasicBlock::iterator From,
+ SmallDenseMap<BasicBlock *, OffsetSpan, 8> &VisitedBlocks,
+ unsigned &ScannedInstCount);
+ OffsetSpan combineOffsetRange(OffsetSpan LHS, OffsetSpan RHS);
+ OffsetSpan computeImpl(Value *V);
+ OffsetSpan computeValue(Value *V);
bool CheckedZextOrTrunc(APInt &I);
};
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index 1edc51e9ce5da3..44cb2d0942f460 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -700,10 +700,21 @@ ObjectSizeOffsetVisitor::ObjectSizeOffsetVisitor(const DataLayout &DL,
SizeOffsetAPInt ObjectSizeOffsetVisitor::compute(Value *V) {
InstructionsVisited = 0;
- return computeImpl(V);
+ OffsetSpan Span = computeImpl(V);
+
+ // In ExactSizeFromOffset mode, we don't care about the Before Field, so allow
+ // us to overwrite it if needs be.
+ if (Span.knownAfter() && !Span.knownBefore() &&
+ Options.EvalMode == ObjectSizeOpts::Mode::ExactSizeFromOffset)
+ Span.Before = APInt::getZero(Span.After.getBitWidth());
+
+ if (!Span.bothKnown())
+ return {};
+
+ return {Span.Before + Span.After, Span.Before};
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
+OffsetSpan ObjectSizeOffsetVisitor::computeImpl(Value *V) {
unsigned InitialIntTyBits = DL.getIndexTypeSizeInBits(V->getType());
// Stripping pointer casts can strip address space casts which can change the
@@ -720,28 +731,28 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeImpl(Value *V) {
IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
Zero = APInt::getZero(IntTyBits);
- SizeOffsetAPInt SOT = computeValue(V);
+ OffsetSpan ORT = computeValue(V);
bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
if (!IndexTypeSizeChanged && Offset.isZero())
- return SOT;
+ return ORT;
// We stripped an address space cast that changed the index type size or we
// accumulated some constant offset (or both). Readjust the bit width to match
// the argument index type size and apply the offset, as required.
if (IndexTypeSizeChanged) {
- if (SOT.knownSize() && !::CheckedZextOrTrunc(SOT.Size, InitialIntTyBits))
- SOT.Size = APInt();
- if (SOT.knownOffset() &&
- !::CheckedZextOrTrunc(SOT.Offset, InitialIntTyBits))
- SOT.Offset = APInt();
+ if (ORT.knownBefore() &&
+ !::CheckedZextOrTrunc(ORT.Before, InitialIntTyBits))
+ ORT.Before = APInt();
+ if (ORT.knownAfter() && !::CheckedZextOrTrunc(ORT.After, InitialIntTyBits))
+ ORT.After = APInt();
}
- // If the computed offset is "unknown" we cannot add the stripped offset.
- return {SOT.Size,
- SOT.Offset.getBitWidth() > 1 ? SOT.Offset + Offset : SOT.Offset};
+ // If the computed bound is "unknown" we cannot add the stripped offset.
+ return {(ORT.knownBefore() ? ORT.Before + Offset : ORT.Before),
+ (ORT.knownAfter() ? ORT.After - Offset : ORT.After)};
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::computeValue(Value *V) {
+OffsetSpan ObjectSizeOffsetVisitor::computeValue(Value *V) {
if (Instruction *I = dyn_cast<Instruction>(V)) {
// If we have already seen this instruction, bail out. Cycles can happen in
// unreachable code after constant propagation.
@@ -751,7 +762,7 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::computeValue(Value *V) {
++InstructionsVisited;
if (InstructionsVisited > ObjectSizeOffsetVisitorMaxVisitInstructions)
return ObjectSizeOffsetVisitor::unknown();
- SizeOffsetAPInt Res = visit(*I);
+ OffsetSpan Res = visit(*I);
// Cache the result for later visits. If we happened to visit this during
// the above recursion, we would consider it unknown until now.
SeenInsts[I] = Res;
@@ -777,13 +788,13 @@ bool ObjectSizeOffsetVisitor::CheckedZextOrTrunc(APInt &I) {
return ::CheckedZextOrTrunc(I, IntTyBits);
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitAllocaInst(AllocaInst &I) {
+OffsetSpan ObjectSizeOffsetVisitor::visitAllocaInst(AllocaInst &I) {
TypeSize ElemSize = DL.getTypeAllocSize(I.getAllocatedType());
if (ElemSize.isScalable() && Options.EvalMode != ObjectSizeOpts::Mode::Min)
return ObjectSizeOffsetVisitor::unknown();
APInt Size(IntTyBits, ElemSize.getKnownMinValue());
if (!I.isArrayAllocation())
- return SizeOffsetAPInt(align(Size, I.getAlign()), Zero);
+ return OffsetSpan(Zero, align(Size, I.getAlign()));
Value *ArraySize = I.getArraySize();
if (const ConstantInt *C = dyn_cast<ConstantInt>(ArraySize)) {
@@ -794,12 +805,12 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::visitAllocaInst(AllocaInst &I) {
bool Overflow;
Size = Size.umul_ov(NumElems, Overflow);
return Overflow ? ObjectSizeOffsetVisitor::unknown()
- : SizeOffsetAPInt(align(Size, I.getAlign()), Zero);
+ : OffsetSpan(Zero, align(Size, I.getAlign()));
}
return ObjectSizeOffsetVisitor::unknown();
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitArgument(Argument &A) {
+OffsetSpan ObjectSizeOffsetVisitor::visitArgument(Argument &A) {
Type *MemoryTy = A.getPointeeInMemoryValueType();
// No interprocedural analysis is done at the moment.
if (!MemoryTy|| !MemoryTy->isSized()) {
@@ -808,16 +819,16 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::visitArgument(Argument &A) {
}
APInt Size(IntTyBits, DL.getTypeAllocSize(MemoryTy));
- return SizeOffsetAPInt(align(Size, A.getParamAlign()), Zero);
+ return OffsetSpan(Zero, align(Size, A.getParamAlign()));
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitCallBase(CallBase &CB) {
+OffsetSpan ObjectSizeOffsetVisitor::visitCallBase(CallBase &CB) {
if (std::optional<APInt> Size = getAllocSize(&CB, TLI))
- return SizeOffsetAPInt(*Size, Zero);
+ return OffsetSpan(Zero, *Size);
return ObjectSizeOffsetVisitor::unknown();
}
-SizeOffsetAPInt
+OffsetSpan
ObjectSizeOffsetVisitor::visitConstantPointerNull(ConstantPointerNull &CPN) {
// If null is unknown, there's nothing we can do. Additionally, non-zero
// address spaces can make use of null, so we don't presume to know anything
@@ -828,45 +839,43 @@ ObjectSizeOffsetVisitor::visitConstantPointerNull(ConstantPointerNull &CPN) {
// addrspace(1) gets casted to addrspace(0) (or vice-versa).
if (Options.NullIsUnknownSize || CPN.getType()->getAddressSpace())
return ObjectSizeOffsetVisitor::unknown();
- return SizeOffsetAPInt(Zero, Zero);
+ return OffsetSpan(Zero, Zero);
}
-SizeOffsetAPInt
+OffsetSpan
ObjectSizeOffsetVisitor::visitExtractElementInst(ExtractElementInst &) {
return ObjectSizeOffsetVisitor::unknown();
}
-SizeOffsetAPInt
-ObjectSizeOffsetVisitor::visitExtractValueInst(ExtractValueInst &) {
+OffsetSpan ObjectSizeOffsetVisitor::visitExtractValueInst(ExtractValueInst &) {
// Easy cases were already folded by previous passes.
return ObjectSizeOffsetVisitor::unknown();
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitGlobalAlias(GlobalAlias &GA) {
+OffsetSpan ObjectSizeOffsetVisitor::visitGlobalAlias(GlobalAlias &GA) {
if (GA.isInterposable())
return ObjectSizeOffsetVisitor::unknown();
return computeImpl(GA.getAliasee());
}
-SizeOffsetAPInt
-ObjectSizeOffsetVisitor::visitGlobalVariable(GlobalVariable &GV) {
+OffsetSpan ObjectSizeOffsetVisitor::visitGlobalVariable(GlobalVariable &GV) {
if (!GV.getValueType()->isSized() || GV.hasExternalWeakLinkage() ||
((!GV.hasInitializer() || GV.isInterposable()) &&
Options.EvalMode != ObjectSizeOpts::Mode::Min))
return ObjectSizeOffsetVisitor::unknown();
APInt Size(IntTyBits, DL.getTypeAllocSize(GV.getValueType()));
- return SizeOffsetAPInt(align(Size, GV.getAlign()), Zero);
+ return OffsetSpan(Zero, align(Size, GV.getAlign()));
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitIntToPtrInst(IntToPtrInst &) {
+OffsetSpan ObjectSizeOffsetVisitor::visitIntToPtrInst(IntToPtrInst &) {
// clueless
return ObjectSizeOffsetVisitor::unknown();
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::findLoadSizeOffset(
+OffsetSpan ObjectSizeOffsetVisitor::findLoadOffsetRange(
LoadInst &Load, BasicBlock &BB, BasicBlock::iterator From,
- SmallDenseMap<BasicBlock *, SizeOffsetAPInt, 8> &VisitedBlocks,
+ SmallDenseMap<BasicBlock *, OffsetSpan, 8> &VisitedBlocks,
unsigned &ScannedInstCount) {
constexpr unsigned MaxInstsToScan = 128;
@@ -877,7 +886,7 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::findLoadSizeOffset(
auto Unknown = [&BB, &VisitedBlocks]() {
return VisitedBlocks[&BB] = ObjectSizeOffsetVisitor::unknown();
};
- auto Known = [&BB, &VisitedBlocks](SizeOffsetAPInt SO) {
+ auto Known = [&BB, &VisitedBlocks](OffsetSpan SO) {
return VisitedBlocks[&BB] = SO;
};
@@ -948,15 +957,15 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::findLoadSizeOffset(
if (!C)
return Unknown();
- return Known({C->getValue(), APInt(C->getValue().getBitWidth(), 0)});
+ return Known({APInt(C->getValue().getBitWidth(), 0), C->getValue()});
}
return Unknown();
} while (From-- != BB.begin());
- SmallVector<SizeOffsetAPInt> PredecessorSizeOffsets;
+ SmallVector<OffsetSpan> PredecessorSizeOffsets;
for (auto *PredBB : predecessors(&BB)) {
- PredecessorSizeOffsets.push_back(findLoadSizeOffset(
+ PredecessorSizeOffsets.push_back(findLoadOffsetRange(
Load, *PredBB, BasicBlock::iterator(PredBB->getTerminator()),
VisitedBlocks, ScannedInstCount));
if (!PredecessorSizeOffsets.back().bothKnown())
@@ -968,70 +977,70 @@ SizeOffsetAPInt ObjectSizeOffsetVisitor::findLoadSizeOffset(
return Known(std::accumulate(
PredecessorSizeOffsets.begin() + 1, PredecessorSizeOffsets.end(),
- PredecessorSizeOffsets.front(),
- [this](SizeOffsetAPInt LHS, SizeOffsetAPInt RHS) {
- return combineSizeOffset(LHS, RHS);
+ PredecessorSizeOffsets.front(), [this](OffsetSpan LHS, OffsetSpan RHS) {
+ return combineOffsetRange(LHS, RHS);
}));
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitLoadInst(LoadInst &LI) {
+OffsetSpan ObjectSizeOffsetVisitor::visitLoadInst(LoadInst &LI) {
if (!Options.AA) {
++ObjectVisitorLoad;
return ObjectSizeOffsetVisitor::unknown();
}
- SmallDenseMap<BasicBlock *, SizeOffsetAPInt, 8> VisitedBlocks;
+ SmallDenseMap<BasicBlock *, OffsetSpan, 8> VisitedBlocks;
unsigned ScannedInstCount = 0;
- SizeOffsetAPInt SO =
- findLoadSizeOffset(LI, *LI.getParent(), BasicBlock::iterator(LI),
- VisitedBlocks, ScannedInstCount);
+ OffsetSpan SO =
+ findLoadOffsetRange(LI, *LI.getParent(), BasicBlock::iterator(LI),
+ VisitedBlocks, ScannedInstCount);
if (!SO.bothKnown())
++ObjectVisitorLoad;
return SO;
}
-SizeOffsetAPInt
-ObjectSizeOffsetVisitor::combineSizeOffset(SizeOffsetAPInt LHS,
- SizeOffsetAPInt RHS) {
+OffsetSpan ObjectSizeOffsetVisitor::combineOffsetRange(OffsetSpan LHS,
+ OffsetSpan RHS) {
if (!LHS.bothKnown() || !RHS.bothKnown())
return ObjectSizeOffsetVisitor::unknown();
switch (Options.EvalMode) {
case ObjectSizeOpts::Mode::Min:
- return (getSizeWithOverflow(LHS).slt(getSizeWithOverflow(RHS))) ? LHS : RHS;
- case ObjectSizeOpts::Mode::Max:
- return (getSizeWithOverflow(LHS).sgt(getSizeWithOverflow(RHS))) ? LHS : RHS;
+ return {LHS.Before.slt(RHS.Before) ? LHS.Before : RHS.Before,
+ LHS.After.slt(RHS.After) ? LHS.After : RHS.After};
+ case ObjectSizeOpts::Mode::Max: {
+ return {LHS.Before.sgt(RHS.Before) ? LHS.Before : RHS.Before,
+ LHS.After.sgt(RHS.After) ? LHS.After : RHS.After};
+ }
case ObjectSizeOpts::Mode::ExactSizeFromOffset:
- return (getSizeWithOverflow(LHS).eq(getSizeWithOverflow(RHS)))
- ? LHS
- : ObjectSizeOffsetVisitor::unknown();
+ return {LHS.Before.eq(RHS.Before) ? LHS.Before : APInt(),
+ LHS.After.eq(RHS.After) ? LHS.After : APInt()};
case ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset:
- return LHS == RHS ? LHS : ObjectSizeOffsetVisitor::unknown();
+ return (LHS == RHS) ? LHS : ObjectSizeOffsetVisitor::unknown();
}
llvm_unreachable("missing an eval mode");
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitPHINode(PHINode &PN) {
+OffsetSpan ObjectSizeOffsetVisitor::visitPHINode(PHINode &PN) {
if (PN.getNumIncomingValues() == 0)
return ObjectSizeOffsetVisitor::unknown();
auto IncomingValues = PN.incoming_values();
return std::accumulate(IncomingValues.begin() + 1, IncomingValues.end(),
computeImpl(*IncomingValues.begin()),
- [this](SizeOffsetAPInt LHS, Value *VRHS) {
- return combineSizeOffset(LHS, computeImpl(VRHS));
+ [this](OffsetSpan LHS, Value *VRHS) {
+ return combineOffsetRange(LHS, computeImpl(VRHS));
});
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitSelectInst(SelectInst &I) {
- return combineSizeOffset(computeImpl(I.getTrueValue()),
- computeImpl(I.getFalseValue()));
+OffsetSpan ObjectSizeOffsetVisitor::visitSelectInst(SelectInst &I) {
+ return combineOffsetRange(computeImpl(I.getTrueValue()),
+ computeImpl(I.getFalseValue()));
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitUndefValue(UndefValue &) {
- return SizeOffsetAPInt(Zero, Zero);
+OffsetSpan ObjectSizeOffsetVisitor::visitUndefValue(UndefValue &) {
+ return OffsetSpan(Zero, Zero);
}
-SizeOffsetAPInt ObjectSizeOffsetVisitor::visitInstruction(Instruction &I) {
+OffsetSpan ObjectSizeOffsetVisitor::visitInstruction(Instruction &I) {
LLVM_DEBUG(dbgs() << "ObjectSizeOffsetVisitor unknown instruction:" << I
<< '\n');
return ObjectSizeOffsetVisitor::unknown();
@@ -1084,7 +1093,13 @@ SizeOffsetValue ObjectSizeOffsetEvaluator::compute(Value *V) {
}
SizeOffsetValue ObjectSizeOffsetEvaluator::compute_(Value *V) {
- ObjectSizeOffsetVisitor Visitor(DL, TLI, Context, EvalOpts);
+
+ // Only trust ObjectSizeOffsetVisitor in exact mode, otherwise fallback on
+ // dynamic computation.
+ ObjectSizeOpts VisitorEvalOpts(EvalOpts);
+ VisitorEvalOpts.EvalMode = ObjectSizeOpts::Mode::ExactUnderlyingSizeAndOffset;
+ ObjectSizeOffsetVisitor Visitor(DL, TLI, Context, VisitorEvalOpts);
+
SizeOffsetAPInt Const = Visitor.compute(V);
if (Const.bothKnown())
return SizeOffsetValue(ConstantInt::get(Context, Const.Size),
diff --git a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
index 4f4d6a88e1693b..2974228e6a8303 100644
--- a/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
+++ b/llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll
@@ -117,3 +117,257 @@ if.end:
%size = call i64 @llvm.objectsize.i64.p0(ptr %p, i1 true, i1 true, i1 false)
ret i64 %size
}
+
+define i64 @pick_negative_offset(i32 %n) {
+; CHECK-LABEL: @pick_negative_offset(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[BUFFER0:%.*]] = alloca i8, i64 20, align 1
+; CHECK-NEXT: [[OFFSETED0:%.*]] = getelementptr i8, ptr [[BUFFER0]], i64 20
+; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[N:%.*]], 0
+; CHECK-NEXT: br i1 [[COND]], label [[IF_ELSE:%.*]], label [[IF_END:%.*]]
+; CHECK: if.else:
+; CHECK-NEXT: [[BUFFER1:%.*]] = alloca i8, i64 20, align 1
+; CHECK-NEXT: [[OFFSETED1:%.*]] = getelementptr i8, ptr [[BUFFER1]], i64 20
+; CHECK-NEXT: br label [[IF_END]]
+; CHECK: if.end:
+; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[OFFSETED1]], [[IF_ELSE]] ], [ [[OFFSETED0]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[POFFSETED:%.*]] = getelementptr i8, ptr [[P]], i64 -4
+; CHECK-NEXT: ret i64 4
+;
+entry:
+ %buffer0 = alloca i8, i64 20
+ %offseted0 = getelementptr i8, ptr %buffer0, i64 20
+ %cond = icmp eq i32 %n, 0
+ br i1 %cond, label %if.else, label %if.end
...
[truncated]
|
|
The But based on that discussion: this PR is also technically an ABI break. It breaks symbols that are public for technical reasons, but are meant to only be used by LLVM internally. What is the policy on that? Is that something that can go into |
|
Who can review this? @nikic? I am very hesitant to merge more stuff that changes the public headers. But I want to hear what other people say. |
|
Yeah, I don't think we should backport this. A backportable variant of this fix would be to disable the phi handling entirely. |
|
Disabling the phi handling entirely would work, but it's been in for a few releases already so I worry it would cause more damage to disable it again, especially if it is only for a single release. I think I can change this PR to avoid changes to public headers. |
llvm#111827) …and Select/Phi When picking a SizeOffsetAPInt through combineSizeOffset, the behavior differs if we're going to apply a constant offset that's positive or negative: If it's positive, then we need to compare the remaining bytes (i.e. Size - Offset), but if it's negative, we need to compare the preceding bytes (i.e. Offset). Fix llvm#111709 (cherry picked from commit 01a103b)
|
This more limited version is not enough to completely fix the incorrect results in LLVM internally, but should be a strict improvement compared to what is currently on Edit: Actually, with one more change, if we treat |
|
Also cc @serge-sans-paille for the chance to comment since this now includes changes of mine and is no longer a straightforward backport. |
|
For the record, I've just submitted #115504 that fixes a few issues related to the same kind of issues... |
|
What's the current state of this? Is this one ready to be merged or should we merge the other PR that @serge-sans-paille linked to? @nikic what's your thoughts? |
|
It would be good if @serge-sans-paille can review this backport, as it's substantially different from the patch on main, and he's the most familiar with this code. The other PR linked above is, as I understand it, not a replacement for this one -- it makes additional fixes in the same area. |
|
@serge-sans-paille what do you think? should this be backported? |
|
I will close this since we haven't gotten any real feedback on the right fix and we are closing out 19.x releases. If you still think it should be fixed for 19 - please reopen this. |
|
I do still think it should be fixed but if any fix needs to be reviewed by @serge-sans-paille and he is not going to review it, there is very little I can do, so might as well leave it closed. |
|
I think the right way forward is to make sure it's fixed in main for LLVM 20. |
|
The reason for the backport of the minimal fix was that LLVM 19 was throwing false |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 01a103b
Requested by: @hvdijk