[llvm] Use computeConstantRange to improve llvm.objectsize computation #114673
Conversation
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: None (serge-sans-paille) ChangesUsing LazyValueInfo, it is possible to compute valuable information for allocation functions, GEP and alloca, even in the presence of dynamic information. llvm.objectsize plays an important role in _FORTIFY_SOURCE definitions, so improving its diagnostic in turns improves the security of compiled application. As a side note, as a result of recent optimization improvements, clang no longer passes https://github.com/serge-sans-paille/builtin_object_size-test-suite This commit restores the situation and greatly improves the scope of code handled by the static version of __builtin_object_size. Patch is 46.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114673.diff 17 Files Affected:
diff --git a/llvm/include/llvm/Analysis/MemoryBuiltins.h b/llvm/include/llvm/Analysis/MemoryBuiltins.h
index c3b11cdf5cf5db..feb4bc7b8c98da 100644
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -42,6 +42,7 @@ class IntegerType;
class IntrinsicInst;
class IntToPtrInst;
class LLVMContext;
+class LazyValueInfo;
class LoadInst;
class PHINode;
class SelectInst;
@@ -160,8 +161,10 @@ struct ObjectSizeOpts {
/// though they can't be evaluated. Otherwise, null is always considered to
/// point to a 0 byte region of memory.
bool NullIsUnknownSize = false;
- /// If set, used for more accurate evaluation
+ /// If set, used for more accurate evaluation.
AAResults *AA = nullptr;
+ /// If set, used for more accurate evaluation.
+ LazyValueInfo *LVI = nullptr;
};
/// Compute the size of the object pointed by Ptr. Returns true and the
@@ -186,6 +189,12 @@ Value *lowerObjectSizeCall(
const TargetLibraryInfo *TLI, AAResults *AA, bool MustSucceed,
SmallVectorImpl<Instruction *> *InsertedInstructions = nullptr);
+Value *lowerObjectSizeCall(
+ IntrinsicInst *ObjectSize, const DataLayout &DL,
+ const TargetLibraryInfo *TLI, AAResults *AA, LazyValueInfo *LVI,
+ bool MustSucceed,
+ SmallVectorImpl<Instruction *> *InsertedInstructions = nullptr);
+
/// SizeOffsetType - A base template class for the object size visitors. Used
/// here as a self-documenting way to handle the values rather than using a
/// \p std::pair.
@@ -275,6 +284,7 @@ class ObjectSizeOffsetVisitor
OffsetSpan visitExtractValueInst(ExtractValueInst &I);
OffsetSpan visitGlobalAlias(GlobalAlias &GA);
OffsetSpan visitGlobalVariable(GlobalVariable &GV);
+ OffsetSpan visitGetElementPtr(GetElementPtrInst &GEP);
OffsetSpan visitIntToPtrInst(IntToPtrInst &);
OffsetSpan visitLoadInst(LoadInst &I);
OffsetSpan visitPHINode(PHINode &);
diff --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
index 3075b7ebae59e6..72861684a7f754 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
@@ -37,6 +37,7 @@ namespace llvm {
class AAResults;
class AssumptionCache;
class OptimizationRemarkEmitter;
+class LazyValueInfo;
class ProfileSummaryInfo;
class TargetLibraryInfo;
class TargetTransformInfo;
@@ -72,6 +73,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
// Required analyses.
AssumptionCache &AC;
TargetLibraryInfo &TLI;
+ LazyValueInfo &LVI;
DominatorTree &DT;
const DataLayout &DL;
SimplifyQuery SQ;
@@ -101,14 +103,15 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
InstCombiner(InstructionWorklist &Worklist, BuilderTy &Builder,
bool MinimizeSize, AAResults *AA, AssumptionCache &AC,
TargetLibraryInfo &TLI, TargetTransformInfo &TTI,
- DominatorTree &DT, OptimizationRemarkEmitter &ORE,
- BlockFrequencyInfo *BFI, BranchProbabilityInfo *BPI,
- ProfileSummaryInfo *PSI, const DataLayout &DL,
+ LazyValueInfo &LVI, DominatorTree &DT,
+ OptimizationRemarkEmitter &ORE, BlockFrequencyInfo *BFI,
+ BranchProbabilityInfo *BPI, ProfileSummaryInfo *PSI,
+ const DataLayout &DL,
ReversePostOrderTraversal<BasicBlock *> &RPOT)
: TTIForTargetIntrinsicsOnly(TTI), Builder(Builder), Worklist(Worklist),
- MinimizeSize(MinimizeSize), AA(AA), AC(AC), TLI(TLI), DT(DT), DL(DL),
- SQ(DL, &TLI, &DT, &AC, nullptr, /*UseInstrInfo*/ true,
- /*CanUseUndef*/ true, &DC),
+ MinimizeSize(MinimizeSize), AA(AA), AC(AC), TLI(TLI), LVI(LVI), DT(DT),
+ DL(DL), SQ(DL, &TLI, &DT, &AC, nullptr, /*UseInstrInfo*/ true,
+ /*CanUseUndef*/ true, &DC),
ORE(ORE), BFI(BFI), BPI(BPI), PSI(PSI), RPOT(RPOT) {}
virtual ~InstCombiner() = default;
diff --git a/llvm/include/llvm/Transforms/Scalar/LowerConstantIntrinsics.h b/llvm/include/llvm/Transforms/Scalar/LowerConstantIntrinsics.h
index 3d4d1034c0b24d..86f542109c1cf0 100644
--- a/llvm/include/llvm/Transforms/Scalar/LowerConstantIntrinsics.h
+++ b/llvm/include/llvm/Transforms/Scalar/LowerConstantIntrinsics.h
@@ -22,9 +22,10 @@ namespace llvm {
class DominatorTree;
class Function;
class TargetLibraryInfo;
+class LazyValueInfo;
bool lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
- DominatorTree *DT);
+ DominatorTree *DT, LazyValueInfo *LVI);
struct LowerConstantIntrinsicsPass :
PassInfoMixin<LowerConstantIntrinsicsPass> {
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index 71400ac46bdcbf..a934fb7cc52558 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -16,6 +16,8 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Analysis/AliasAnalysis.h"
+#include "llvm/Analysis/AssumptionCache.h"
+#include "llvm/Analysis/LazyValueInfo.h"
#include "llvm/Analysis/TargetFolder.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/Analysis/Utils/Local.h"
@@ -590,19 +592,28 @@ Value *llvm::lowerObjectSizeCall(IntrinsicInst *ObjectSize,
const TargetLibraryInfo *TLI,
bool MustSucceed) {
return lowerObjectSizeCall(ObjectSize, DL, TLI, /*AAResults=*/nullptr,
- MustSucceed);
+ /*LazyValueInfo=*/nullptr, MustSucceed);
}
Value *llvm::lowerObjectSizeCall(
IntrinsicInst *ObjectSize, const DataLayout &DL,
const TargetLibraryInfo *TLI, AAResults *AA, bool MustSucceed,
SmallVectorImpl<Instruction *> *InsertedInstructions) {
+ return lowerObjectSizeCall(ObjectSize, DL, TLI, AA, /*LazyValueInfo=*/nullptr,
+ MustSucceed, InsertedInstructions);
+}
+
+Value *llvm::lowerObjectSizeCall(
+ IntrinsicInst *ObjectSize, const DataLayout &DL,
+ const TargetLibraryInfo *TLI, AAResults *AA, LazyValueInfo *LVI,
+ bool MustSucceed, SmallVectorImpl<Instruction *> *InsertedInstructions) {
assert(ObjectSize->getIntrinsicID() == Intrinsic::objectsize &&
"ObjectSize must be a call to llvm.objectsize!");
bool MaxVal = cast<ConstantInt>(ObjectSize->getArgOperand(1))->isZero();
ObjectSizeOpts EvalOptions;
EvalOptions.AA = AA;
+ EvalOptions.LVI = LVI;
// Unless we have to fold this to something, try to be as accurate as
// possible.
@@ -716,7 +727,6 @@ OffsetSpan ObjectSizeOffsetVisitor::computeImpl(Value *V) {
// value that is passed to computeImpl.
IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
Zero = APInt::getZero(IntTyBits);
-
OffsetSpan ORT = computeValue(V);
bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
@@ -794,6 +804,17 @@ OffsetSpan ObjectSizeOffsetVisitor::visitAllocaInst(AllocaInst &I) {
Size = Size.umul_ov(NumElems, Overflow);
return Overflow ? ObjectSizeOffsetVisitor::unknown()
: OffsetSpan(Zero, align(Size, I.getAlign()));
+ } else if (Options.LVI) {
+ ConstantRange CR =
+ Options.LVI->getConstantRange(const_cast<Value *>(ArraySize), &I, true);
+ if (CR.isFullSet())
+ return ObjectSizeOffsetVisitor::unknown();
+ APInt Bound;
+ if (Options.EvalMode == ObjectSizeOpts::Mode::Max)
+ Bound = CR.getUnsignedMax();
+ else
+ Bound = CR.getUnsignedMin();
+ return OffsetSpan(Zero, align(Bound, I.getAlign()));
}
return ObjectSizeOffsetVisitor::unknown();
}
@@ -811,7 +832,23 @@ OffsetSpan ObjectSizeOffsetVisitor::visitArgument(Argument &A) {
}
OffsetSpan ObjectSizeOffsetVisitor::visitCallBase(CallBase &CB) {
- if (std::optional<APInt> Size = getAllocSize(&CB, TLI))
+ if (std::optional<APInt> Size =
+ getAllocSize(&CB, TLI, [&CB, this](const Value *V) -> const Value * {
+ if (!Options.LVI)
+ return V;
+ if (!V->getType()->isIntegerTy())
+ return V;
+ ConstantRange CR = Options.LVI->getConstantRange(
+ const_cast<Value *>(V), &CB, true);
+ if (CR.isFullSet())
+ return V;
+ APInt Bound;
+ if (Options.EvalMode == ObjectSizeOpts::Mode::Max)
+ Bound = CR.getUnsignedMax();
+ else
+ Bound = CR.getUnsignedMin();
+ return ConstantInt::get(V->getType(), Bound);
+ }))
return OffsetSpan(Zero, *Size);
return ObjectSizeOffsetVisitor::unknown();
}
@@ -856,6 +893,42 @@ OffsetSpan ObjectSizeOffsetVisitor::visitGlobalVariable(GlobalVariable &GV) {
return OffsetSpan(Zero, align(Size, GV.getAlign()));
}
+OffsetSpan ObjectSizeOffsetVisitor::visitGetElementPtr(GetElementPtrInst &GEP) {
+ OffsetSpan PtrData = computeImpl(GEP.getPointerOperand());
+ if (!PtrData.bothKnown())
+ return ObjectSizeOffsetVisitor::unknown();
+
+ if (!Options.LVI)
+ return ObjectSizeOffsetVisitor::unknown();
+
+ if (Options.EvalMode == ObjectSizeOpts::Mode::Min ||
+ Options.EvalMode == ObjectSizeOpts::Mode::Max) {
+ unsigned BitWidth = PtrData.After.getBitWidth();
+ APInt ConstantOffset = Zero;
+ SmallMapVector<Value *, APInt, 4> VariableOffsets;
+ if (!GEP.collectOffset(DL, BitWidth, VariableOffsets, ConstantOffset))
+ return ObjectSizeOffsetVisitor::unknown();
+
+ ConstantRange AccumulatedRange = ConstantOffset;
+ for (auto const &VO : VariableOffsets) {
+ ConstantRange CR = Options.LVI->getConstantRange(VO.first, &GEP, true);
+ if (CR.isFullSet())
+ return ObjectSizeOffsetVisitor::unknown();
+
+ AccumulatedRange = AccumulatedRange.add(CR.multiply(VO.second));
+ }
+
+ APInt Bound;
+ if (Options.EvalMode == ObjectSizeOpts::Mode::Min)
+ Bound = AccumulatedRange.getSignedMax();
+ else
+ Bound = AccumulatedRange.getSignedMin();
+
+ return {PtrData.Before + Bound, PtrData.After - Bound};
+ }
+ return ObjectSizeOffsetVisitor::unknown();
+}
+
OffsetSpan ObjectSizeOffsetVisitor::visitIntToPtrInst(IntToPtrInst &) {
// clueless
return ObjectSizeOffsetVisitor::unknown();
diff --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
index 3373b76edb268f..ba42af6151e27b 100644
--- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -350,7 +350,8 @@ bool PreISelIntrinsicLowering::lowerIntrinsics(Module &M) const {
Function *Parent = CI->getParent()->getParent();
TargetLibraryInfo &TLI = LookupTLI(*Parent);
// Intrinsics in unreachable code are not lowered.
- bool Changed = lowerConstantIntrinsics(*Parent, TLI, /*DT=*/nullptr);
+ bool Changed = lowerConstantIntrinsics(*Parent, TLI, /*DT=*/nullptr,
+ /*LVI=*/nullptr);
return Changed;
});
break;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index adbd9186c59c5a..820be6fc2bb1f6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -63,12 +63,13 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
InstCombinerImpl(InstructionWorklist &Worklist, BuilderTy &Builder,
bool MinimizeSize, AAResults *AA, AssumptionCache &AC,
TargetLibraryInfo &TLI, TargetTransformInfo &TTI,
- DominatorTree &DT, OptimizationRemarkEmitter &ORE,
- BlockFrequencyInfo *BFI, BranchProbabilityInfo *BPI,
- ProfileSummaryInfo *PSI, const DataLayout &DL,
+ LazyValueInfo &LVI, DominatorTree &DT,
+ OptimizationRemarkEmitter &ORE, BlockFrequencyInfo *BFI,
+ BranchProbabilityInfo *BPI, ProfileSummaryInfo *PSI,
+ const DataLayout &DL,
ReversePostOrderTraversal<BasicBlock *> &RPOT)
- : InstCombiner(Worklist, Builder, MinimizeSize, AA, AC, TLI, TTI, DT, ORE,
- BFI, BPI, PSI, DL, RPOT) {}
+ : InstCombiner(Worklist, Builder, MinimizeSize, AA, AC, TLI, TTI, LVI, DT,
+ ORE, BFI, BPI, PSI, DL, RPOT) {}
virtual ~InstCombinerImpl() = default;
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 2a54390c0f1882..9466d17272888c 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -48,6 +48,7 @@
#include "llvm/Analysis/GlobalsModRef.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LazyBlockFrequencyInfo.h"
+#include "llvm/Analysis/LazyValueInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/Analysis/ProfileSummaryInfo.h"
@@ -3317,8 +3318,9 @@ Instruction *InstCombinerImpl::visitAllocSite(Instruction &MI) {
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
if (II->getIntrinsicID() == Intrinsic::objectsize) {
SmallVector<Instruction *> InsertedInstructions;
- Value *Result = lowerObjectSizeCall(
- II, DL, &TLI, AA, /*MustSucceed=*/true, &InsertedInstructions);
+ Value *Result =
+ lowerObjectSizeCall(II, DL, &TLI, AA, &LVI, /*MustSucceed=*/true,
+ &InsertedInstructions);
for (Instruction *Inserted : InsertedInstructions)
Worklist.add(Inserted);
replaceInstUsesWith(*I, Result);
@@ -5460,9 +5462,9 @@ void InstCombiner::computeBackEdges() {
static bool combineInstructionsOverFunction(
Function &F, InstructionWorklist &Worklist, AliasAnalysis *AA,
AssumptionCache &AC, TargetLibraryInfo &TLI, TargetTransformInfo &TTI,
- DominatorTree &DT, OptimizationRemarkEmitter &ORE, BlockFrequencyInfo *BFI,
- BranchProbabilityInfo *BPI, ProfileSummaryInfo *PSI,
- const InstCombineOptions &Opts) {
+ LazyValueInfo &LVI, DominatorTree &DT, OptimizationRemarkEmitter &ORE,
+ BlockFrequencyInfo *BFI, BranchProbabilityInfo *BPI,
+ ProfileSummaryInfo *PSI, const InstCombineOptions &Opts) {
auto &DL = F.getDataLayout();
bool VerifyFixpoint = Opts.VerifyFixpoint &&
!F.hasFnAttribute("instcombine-no-verify-fixpoint");
@@ -5501,8 +5503,8 @@ static bool combineInstructionsOverFunction(
LLVM_DEBUG(dbgs() << "\n\nINSTCOMBINE ITERATION #" << Iteration << " on "
<< F.getName() << "\n");
- InstCombinerImpl IC(Worklist, Builder, F.hasMinSize(), AA, AC, TLI, TTI, DT,
- ORE, BFI, BPI, PSI, DL, RPOT);
+ InstCombinerImpl IC(Worklist, Builder, F.hasMinSize(), AA, AC, TLI, TTI,
+ LVI, DT, ORE, BFI, BPI, PSI, DL, RPOT);
IC.MaxArraySizeForCombine = MaxArraySize;
bool MadeChangeInThisIteration = IC.prepareWorklist(F);
MadeChangeInThisIteration |= IC.run();
@@ -5552,6 +5554,7 @@ PreservedAnalyses InstCombinePass::run(Function &F,
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
auto &TTI = AM.getResult<TargetIRAnalysis>(F);
+ auto &LVI = AM.getResult<LazyValueAnalysis>(F);
auto *AA = &AM.getResult<AAManager>(F);
auto &MAMProxy = AM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
@@ -5561,8 +5564,8 @@ PreservedAnalyses InstCombinePass::run(Function &F,
&AM.getResult<BlockFrequencyAnalysis>(F) : nullptr;
auto *BPI = AM.getCachedResult<BranchProbabilityAnalysis>(F);
- if (!combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, DT, ORE,
- BFI, BPI, PSI, Options))
+ if (!combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, LVI, DT,
+ ORE, BFI, BPI, PSI, Options))
// No changes, all analyses are preserved.
return PreservedAnalyses::all();
@@ -5580,6 +5583,7 @@ void InstructionCombiningPass::getAnalysisUsage(AnalysisUsage &AU) const {
AU.addRequired<TargetTransformInfoWrapperPass>();
AU.addRequired<DominatorTreeWrapperPass>();
AU.addRequired<OptimizationRemarkEmitterWrapperPass>();
+ AU.addRequired<LazyValueInfoWrapperPass>();
AU.addPreserved<DominatorTreeWrapperPass>();
AU.addPreserved<AAResultsWrapperPass>();
AU.addPreserved<BasicAAWrapperPass>();
@@ -5597,6 +5601,7 @@ bool InstructionCombiningPass::runOnFunction(Function &F) {
auto &AC = getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(F);
auto &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
+ auto &LVI = getAnalysis<LazyValueInfoWrapperPass>().getLVI();
auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
auto &ORE = getAnalysis<OptimizationRemarkEmitterWrapperPass>().getORE();
@@ -5612,8 +5617,9 @@ bool InstructionCombiningPass::runOnFunction(Function &F) {
getAnalysisIfAvailable<BranchProbabilityInfoWrapperPass>())
BPI = &WrapperPass->getBPI();
- return combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, DT, ORE,
- BFI, BPI, PSI, InstCombineOptions());
+ return combineInstructionsOverFunction(F, Worklist, AA, AC, TLI, TTI, LVI, DT,
+ ORE, BFI, BPI, PSI,
+ InstCombineOptions());
}
char InstructionCombiningPass::ID = 0;
@@ -5630,6 +5636,7 @@ INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(LazyValueInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(OptimizationRemarkEmitterWrapperPass)
INITIALIZE_PASS_DEPENDENCY(LazyBlockFrequencyInfoPass)
INITIALIZE_PASS_DEPENDENCY(ProfileSummaryInfoWrapperPass)
diff --git a/llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp b/llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
index 8dd72b8f1414ed..f764752695afdc 100644
--- a/llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
+++ b/llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
@@ -18,6 +18,7 @@
#include "llvm/Analysis/DomTreeUpdater.h"
#include "llvm/Analysis/GlobalsModRef.h"
#include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Analysis/LazyValueInfo.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/BasicBlock.h"
@@ -100,7 +101,7 @@ static bool replaceConditionalBranchesOnConstant(Instruction *II,
}
bool llvm::lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
- DominatorTree *DT) {
+ DominatorTree *DT, LazyValueInfo *LVI) {
std::optional<DomTreeUpdater> DTU;
if (DT)
DTU.emplace(DT, DomTreeUpdater::UpdateStrategy::Lazy);
@@ -144,7 +145,7 @@ bool llvm::lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
IsConstantIntrinsicsHandled++;
break;
case Intrinsic::objectsize:
- NewValue = lowerObjectSizeCall(II, DL, &TLI, true);
+ NewValue = lowerObjectSizeCall(II, DL, &TLI, nullptr, LVI, true);
LLVM_DEBUG(dbgs() << "Folding " << *II << " to " << *NewValue << "\n");
ObjectSizeIntrinsicsHandled++;
break;
@@ -160,7 +161,8 @@ bool llvm::lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
PreservedAnalyses
LowerConstantIntrinsicsPass::run(Function &F, FunctionAnalysisManager &AM) {
if (lowerConstantIntrinsics(F, AM.getResult<TargetLibraryAnalysis>(F),
- AM.getCachedResult<DominatorTreeAnalysis>(F))) {
+ AM.getCachedResult<DominatorTreeAnalysis>(F),
+ &AM.getResult<LazyValueAnalysis>(F))) {
PreservedAnalyses PA;
PA.preserve<DominatorTreeAnalysis>();
return PA;
diff --git a/llvm/test/Other/new-pm-defaults.ll b/llvm/test/Other/new-pm-defaults.ll
index 55dbdb1b8366d6..17211...
[truncated]
|
There was a problem hiding this comment.
We definitely cannot use LVI inside InstCombine for invalidation reasons. Using it in LowerConstantIntrinsics may be theoretically possible, but I really don't want to go there.
Can your motivating cases be covered by ValueTracking helpers, either computeConstantRange() or computeKnownBits()?
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
f64ca30 to
b1277d2
Compare
|
I've just submitted a version that uses
I'm not familiar with this. Thanks! |
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
b1277d2 to
59dd36c
Compare
InstCombine has a lot of different folds, and some of them do things like modifying instructions in place. It's essentially impossible to keep analyses that have non-trivial state up-to-date in InstCombine. |
|
more than 9k extra |
|
It feels a bit risky to use |
|
The problem I hinted at in my previous comment turns out to be a problem already even without your PR. Consider something like int f(int n) {
__builtin_assume(n >= 20);
int buf[n];
buf[10] = 0;
return buf[10];
}
int main(void) {
return f(3);
}It is good that in recent versions of Clang, with It is less good that without that I am not sure what the intended behaviour is here. Is this something we want to diagnose, or is this something we want to optimise away? |
agreed.
It's indeed tricky. From a strict I'll update the patch to not use assumptions, that's already a strict improvement. |
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
59dd36c to
95739c6
Compare
That is a good idea, we can always add assumptions in a later follow-up PR if we do want them. This approach looks good, thanks, just left a small comment about handling overflow but that should not change the main idea of your PR. |
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
95739c6 to
8842c80
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
8842c80 to
5cc9b2e
Compare
There was a problem hiding this comment.
Thanks, looks good to me!
You updated the commit according to the change requested by @nikic, but the PR title and description are still the original, so just make sure that when you merge and squash, you do not accidentally overwrite your correct commit message with the older PR description :)
serge-sans-paille
changed the title
Using computeConstantRange, it is possible to compute valuable information for allocation functions, GEP and alloca, even in the presence of some dynamic information. llvm.objectsize plays an important role in _FORTIFY_SOURCE definitions, so improving its diagnostic in turns improves the security of compiled application. As a side note, as a result of recent optimization improvements, clang no longer passes https://github.com/serge-sans-paille/builtin_object_size-test-suite This commit restores the situation and greatly improves the scope of code handled by the static version of __builtin_object_size.
serge-sans-paille
force-pushed
the
feature/lvi-bos
branch
from
5cc9b2e to
f30f00a
Compare
|
The failing test cases are unrelated, merging. |
|
I did write
for a reason, I have been bit by this before. f30f00a had a correct commit message, the squashed 5f34281 has an incorrect commit message. It is fine, this has happened before and will happen again, but it is good for us to be aware of it. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/8906 Here is the relevant piece of the build log for the reference |
This breakage may be related to the change. The bot has since not recovered and the previously flaky test got disabled. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/8175 Here is the relevant piece of the build log for the reference |
Yeah, I did thought it was ok :-/ |
It's difficult to reproduce as it's related to the hip compiler... can you provide guidance ? |
this one got back to green. |
Yeah, this one is misbehaving sometimes. |
Hello, |
Thank you. |
|
This has broken 2 stage builds on at least PowerPC and AArch64: Making a reproducer for that will be tricky so I suggest reverting this for now. It's possible you can reproduce it by doing a 2 stage build on x86 as well, we just don't appear to have a bot doing that. |
|
+1 to the suggestion to revert for now from my side. |
…mputation (#114673)" This reverts commit 5f34281. This seems to break various builders, such as https://lab.llvm.org/buildbot/#/builders/41/builds/3259 https://lab.llvm.org/buildbot/#/builders/76/builds/4298
|
reverted by f5e4ffa |
|
This also caused some Fortran test suite failures on our single stage bot: https://lab.llvm.org/buildbot/#/builders/143/builds/3281 It's a single stage so the problem shouldn't be the flang binary itself but the code that it produces. It is Fortran so probably less familiar to you, but it would be less code to sift through than all of llvm-tablegen. If you want to reproduce you'd build flang, install it (flang needs some header files that get put in the right place by the install) then compile those files from llvm-test-suite. IIRC Flang supports the If it doesn't reproduce on your machine I can try it on ours but I won't be able to do so until Tuesday. |
|
I've managed to reproduce locally, digging... |
|
A potential fix is available there: #115504 interestingly, the underlying issue is that we cannot correctly represent large unsigned values and negative offsets, so we need to make a choice. |
…mputation (llvm#114673)" This reverts commit 5f34281. This seems to break various builders, such as https://lab.llvm.org/buildbot/#/builders/41/builds/3259 https://lab.llvm.org/buildbot/#/builders/76/builds/4298
Using LazyValueInfo, it is possible to compute valuable information for allocation functions, GEP and alloca, even in the presence of dynamic information.
llvm.objectsize plays an important role in _FORTIFY_SOURCE definitions, so improving its diagnostic in turns improves the security of compiled application.
As a side note, as a result of recent optimization improvements, clang no longer passes https://github.com/serge-sans-paille/builtin_object_size-test-suite This commit restores the situation and greatly improves the scope of code handled by the static version of __builtin_object_size.