Skip to content

[MergeICmps] Perform dereferenceability check with context #202884

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
nikic merged 1 commit into from
Jun 12, 2026
+103 −14
Merged

[MergeICmps] Perform dereferenceability check with context #202884

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
59 changes: 49 additions & 10 deletions llvm/lib/Transforms/Scalar/MergeICmps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);

Expand All @@ -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

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);

@antoniofrighetto antoniofrighetto Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would the nature / invariants of the pass allow us not to particularly care about DT (/AC)-based reasoning here when proving dereferenceability?

@nikic nikic Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In theory AC/DT could be useful to support this transform based on assume_dereferenceable. The pass doesn't use AC/DT currently though (DT is only preserved), and I don't think it's worth it to request them just for this.

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");
Expand Down Expand Up @@ -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);
}

Expand Down
57 changes: 53 additions & 4 deletions llvm/test/Transforms/MergeICmps/X86/no-gep-other-work.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -9,17 +10,65 @@ 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:%.*]]
; CHECK: join:
; 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
Expand Down
1 change: 1 addition & 0 deletions llvm/test/Transforms/MergeICmps/X86/opaque-ptr.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"

Expand Down