[llvm] [MergeICmps] Perform dereferenceability check with context (PR #202884)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 10 01:15:22 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Nikita Popov (nikic)
<details>
<summary>Changes</summary>
To support deref-at-point semantics, we need to check dereferenceability with a context instruction. Currently, MergeICmps does the check for each individual load instruction. In this PR, I'm replacing this with a check for all the loads that are part of a chain after they have been collected, so we do the context-sensitive check only once.
The choice of context instruction is a bit tricky: Normally, this would just be the first block in the chain (the "entry block"), but it's also possible for the block to "do extra work", in which case it will get split. If this happens, we should be checking at the splitting point, as the extra work might be freeing the pointer.
Another question to consider here is whether we need to be concerned about frees at all: After all, the original code will be accessing at least one byte of the two objects, so doesn't that imply that it wasn't freed already? This is indeed the case, as long as allocations cannot shrink. This is something we currently don't allow, but I think it's something we want to allow, so I'm going with the conservative treatment here.
---
Full diff: https://github.com/llvm/llvm-project/pull/202884.diff
3 Files Affected:
- (modified) llvm/lib/Transforms/Scalar/MergeICmps.cpp (+49-10)
- (modified) llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll (+53-4)
- (modified) llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll (+1)
``````````diff
diff --git a/llvm/lib/Transforms/Scalar/MergeICmps.cpp b/llvm/lib/Transforms/Scalar/MergeICmps.cpp
index 4c7c215d3cb3b..f8beaab7c6f5c 100644
--- a/llvm/lib/Transforms/Scalar/MergeICmps.cpp
+++ b/llvm/lib/Transforms/Scalar/MergeICmps.cpp
@@ -162,13 +162,6 @@ static BCEAtom visitICmpLoadOperand(Value *const Val, BaseIdentifier &BaseId) {
return {};
}
- if (!isDereferenceablePointer(Addr, LoadI->getType(), DL)) {
- LLVM_DEBUG(dbgs() << "not dereferenceable\n");
- // We need to make sure that we can do comparison in any order, so we
- // require memory to be unconditionally dereferenceable.
- return {};
- }
-
APInt Offset = APInt(DL.getIndexTypeSizeInBits(Addr->getType()), 0);
Value *Base = Addr;
auto *GEP = dyn_cast<GetElementPtrInst>(Addr);
@@ -225,7 +218,9 @@ class BCECmpBlock {
// Returns true if the non-BCE-cmp instructions can be separated from BCE-cmp
// instructions in the block.
- bool canSplit(AliasAnalysis &AA) const;
+ // SplitAt is set to the instruction before which the block will have to be
+ // split.
+ bool canSplit(AliasAnalysis &AA, Instruction *&SplitAt) const;
// Return true if this all the relevant instructions in the BCE-cmp-block can
// be sunk below this instruction. By doing this, we know we can separate the
@@ -290,9 +285,11 @@ void BCECmpBlock::split(BasicBlock *NewParent, AliasAnalysis &AA) const {
Inst->moveBeforePreserving(*NewParent, NewParent->begin());
}
-bool BCECmpBlock::canSplit(AliasAnalysis &AA) const {
+bool BCECmpBlock::canSplit(AliasAnalysis &AA, Instruction *&SplitAt) const {
+ SplitAt = nullptr;
for (Instruction &Inst : *BB) {
if (!BlockInsts.count(&Inst)) {
+ SplitAt = Inst.getNextNode();
if (!canSinkBCECmpInst(&Inst, AA))
return false;
}
@@ -416,6 +413,8 @@ class BCECmpChain {
BCECmpChain(const std::vector<BasicBlock *> &Blocks, PHINode &Phi,
AliasAnalysis &AA);
+ bool isDereferenceable();
+
bool simplify(const TargetLibraryInfo &TLI, AliasAnalysis &AA,
DomTreeUpdater &DTU);
@@ -430,6 +429,9 @@ class BCECmpChain {
std::vector<ContiguousBlocks> MergedBlocks_;
// The original entry block (before sorting);
BasicBlock *EntryBlock_;
+ // The instruction before which the entry block needs to be split (or null
+ // if no splitting required).
+ Instruction *SplitAt = nullptr;
};
} // namespace
@@ -518,13 +520,14 @@ BCECmpChain::BCECmpChain(const std::vector<BasicBlock *> &Blocks, PHINode &Phi,
// and start anew.
//
// NOTE: we only handle blocks a with single predecessor for now.
- if (Comparison->canSplit(AA)) {
+ if (Comparison->canSplit(AA, SplitAt)) {
LLVM_DEBUG(dbgs()
<< "Split initial block '" << Comparison->BB->getName()
<< "' that does extra work besides compare\n");
Comparison->RequireSplit = true;
enqueueBlock(Comparisons, std::move(*Comparison));
} else {
+ SplitAt = nullptr;
LLVM_DEBUG(dbgs()
<< "ignoring initial block '" << Comparison->BB->getName()
<< "' that does extra work besides compare\n");
@@ -734,6 +737,37 @@ static BasicBlock *mergeComparisons(ArrayRef<BCECmpBlock> Comparisons,
return BB;
}
+// The transform may change the order in which the comparison is performed,
+// in which case we may perform loads that were not performed by the original
+// program. As such, we need to ensure that all the accessed memory is
+// dereferenceable.
+bool BCECmpChain::isDereferenceable() {
+ // We know that there can be no frees inside the merged blocks, so it's
+ // sufficient for dereferenceability to hold at the entry block. One
+ // exception to this is if the entry block performs "other work" and will
+ // get split. In that case, we need to consider frees prior to the splitting
+ // point.
+ Instruction *CxtI = SplitAt ? SplitAt : &EntryBlock_->front();
+
+ for (const auto &Blocks : MergedBlocks_) {
+ const BCECmpBlock &LowestBlock = Blocks.front();
+ const Value *Lhs = LowestBlock.Lhs().LoadI->getPointerOperand();
+ const Value *Rhs = LowestBlock.Rhs().LoadI->getPointerOperand();
+ const DataLayout &DL = LowestBlock.Lhs().LoadI->getDataLayout();
+
+ unsigned SizeInBits = 0;
+ for (const BCECmpBlock &Block : Blocks)
+ SizeInBits += Block.SizeBits();
+
+ APInt Size(64, SizeInBits / 8);
+ SimplifyQuery SQ(DL, CxtI);
+ if (!isDereferenceableAndAlignedPointer(Lhs, Align(1), Size, SQ) ||
+ !isDereferenceableAndAlignedPointer(Rhs, Align(1), Size, SQ))
+ return false;
+ }
+ return true;
+}
+
bool BCECmpChain::simplify(const TargetLibraryInfo &TLI, AliasAnalysis &AA,
DomTreeUpdater &DTU) {
assert(atLeastOneMerged() && "simplifying trivial BCECmpChain");
@@ -888,6 +922,11 @@ static bool processPhi(PHINode &Phi, const TargetLibraryInfo &TLI,
return false;
}
+ if (!CmpChain.isDereferenceable()) {
+ LLVM_DEBUG(dbgs() << "not dereferenceable\n");
+ return false;
+ }
+
return CmpChain.simplify(TLI, AA, DTU);
}
diff --git a/llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll b/llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll
index 0cbc8dce5bd51..71c040a4a0e8e 100644
--- a/llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll
+++ b/llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll
@@ -1,5 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -S -passes=mergeicmps < %s | FileCheck %s
+; RUN: opt -S -passes=mergeicmps < %s | FileCheck %s --check-prefixes=CHECK,GLOBAL
+; RUN: opt -S -passes=mergeicmps -use-dereferenceable-at-point-semantics < %s | FileCheck %s --check-prefixes=CHECK,AT-POINT
; This does not use a GEP for the zero-offset comparison and requires a
; split for other work.
@@ -9,9 +10,57 @@ target triple = "x86_64-grtev4-linux-gnu"
declare void @other_work()
define i1 @test(ptr dereferenceable(2) %arg, ptr dereferenceable(2) %arg1) {
-; CHECK-LABEL: @test(
+; GLOBAL-LABEL: @test(
+; GLOBAL-NEXT: "if+entry":
+; GLOBAL-NEXT: call void @other_work()
+; GLOBAL-NEXT: [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[ARG:%.*]], ptr [[ARG1:%.*]], i64 2)
+; GLOBAL-NEXT: [[TMP0:%.*]] = icmp eq i32 [[MEMCMP]], 0
+; GLOBAL-NEXT: br label [[JOIN:%.*]]
+; GLOBAL: join:
+; GLOBAL-NEXT: ret i1 [[TMP0]]
+;
+; AT-POINT-LABEL: @test(
+; AT-POINT-NEXT: entry:
+; AT-POINT-NEXT: call void @other_work()
+; AT-POINT-NEXT: [[ARG_OFF:%.*]] = getelementptr inbounds i8, ptr [[ARG:%.*]], i64 1
+; AT-POINT-NEXT: [[ARG1_OFF:%.*]] = getelementptr inbounds i8, ptr [[ARG1:%.*]], i64 1
+; AT-POINT-NEXT: [[ARG_OFF_VAL:%.*]] = load i8, ptr [[ARG_OFF]], align 1
+; AT-POINT-NEXT: [[ARG1_OFF_VAL:%.*]] = load i8, ptr [[ARG1_OFF]], align 1
+; AT-POINT-NEXT: [[CMP_OFF:%.*]] = icmp eq i8 [[ARG_OFF_VAL]], [[ARG1_OFF_VAL]]
+; AT-POINT-NEXT: br i1 [[CMP_OFF]], label [[IF:%.*]], label [[JOIN:%.*]]
+; AT-POINT: if:
+; AT-POINT-NEXT: [[ARG_VAL:%.*]] = load i8, ptr [[ARG]], align 1
+; AT-POINT-NEXT: [[ARG1_VAL:%.*]] = load i8, ptr [[ARG1]], align 1
+; AT-POINT-NEXT: [[CMP:%.*]] = icmp eq i8 [[ARG_VAL]], [[ARG1_VAL]]
+; AT-POINT-NEXT: br label [[JOIN]]
+; AT-POINT: join:
+; AT-POINT-NEXT: [[PHI:%.*]] = phi i1 [ false, [[ENTRY:%.*]] ], [ [[CMP]], [[IF]] ]
+; AT-POINT-NEXT: ret i1 [[PHI]]
+;
+entry:
+ call void @other_work()
+ %arg.off = getelementptr inbounds i8, ptr %arg, i64 1
+ %arg1.off = getelementptr inbounds i8, ptr %arg1, i64 1
+ %arg.off.val = load i8, ptr %arg.off
+ %arg1.off.val = load i8, ptr %arg1.off
+ %cmp.off = icmp eq i8 %arg.off.val, %arg1.off.val
+ br i1 %cmp.off, label %if, label %join
+
+if:
+ %arg.val = load i8, ptr %arg
+ %arg1.val = load i8, ptr %arg1
+ %cmp = icmp eq i8 %arg.val, %arg1.val
+ br label %join
+
+join:
+ %phi = phi i1 [ false, %entry ], [ %cmp, %if ]
+ ret i1 %phi
+}
+
+define i1 @test_nofree(ptr dereferenceable(2) %arg, ptr dereferenceable(2) %arg1) {
+; CHECK-LABEL: @test_nofree(
; CHECK-NEXT: "if+entry":
-; CHECK-NEXT: call void @other_work()
+; CHECK-NEXT: call void @other_work() #[[ATTR1:[0-9]+]]
; CHECK-NEXT: [[MEMCMP:%.*]] = call i32 @memcmp(ptr [[ARG:%.*]], ptr [[ARG1:%.*]], i64 2)
; CHECK-NEXT: [[TMP0:%.*]] = icmp eq i32 [[MEMCMP]], 0
; CHECK-NEXT: br label [[JOIN:%.*]]
@@ -19,7 +68,7 @@ define i1 @test(ptr dereferenceable(2) %arg, ptr dereferenceable(2) %arg1) {
; CHECK-NEXT: ret i1 [[TMP0]]
;
entry:
- call void @other_work()
+ call void @other_work() nofree
%arg.off = getelementptr inbounds i8, ptr %arg, i64 1
%arg1.off = getelementptr inbounds i8, ptr %arg1, i64 1
%arg.off.val = load i8, ptr %arg.off
diff --git a/llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll b/llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll
index df2fa0cee08dd..f3ba65759f32a 100644
--- a/llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll
+++ b/llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll
@@ -1,5 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -passes=mergeicmps < %s | FileCheck %s
+; RUN: opt -S -passes=mergeicmps -use-dereferenceable-at-point-semantics < %s | FileCheck %s
target triple = "x86_64-unknown-unknown"
``````````
</details>
https://github.com/llvm/llvm-project/pull/202884
More information about the llvm-commits
mailing list